Pages:
Author

Topic: Nxt source code flaw reports - page 48. (Read 113369 times)

hero member
Activity: 589
Merit: 500
January 05, 2014, 12:10:04 AM
Surely the solution to the flaw entitles me to some NXT as well. Smiley
Yep! Those who see only flaws - they sells. Those who see solutions - we buy.

Double-yep... up until now, Nxt has been my "High Risk" investment of choice.  It's moved up a notch to just a "Risk".

This is because it's new and there is always a chance that this thing could get Luke-Jr'd like many of the early alt-coins. Wink

Interesting~

Luke-Jr'd

We need innovation based on innovation, not stop on an innovation.
hero member
Activity: 784
Merit: 501
January 04, 2014, 11:59:08 PM
byte[] generationSignatureHash = MessageDigest.getInstance("SHA-256").digest(generationSignature); seems should be
byte[] generationSignatureHash = MessageDigest.getInstance("SHA-256").digest(previousBlock.generationSignature);
No. This is hash of signature. Look at famous line 288, and you will see usage of last block gen sig.
newbie
Activity: 26
Merit: 0
January 04, 2014, 11:53:13 PM
Code:
boolean verifyGenerationSignature() {

try {

if (getLastBlock().height <= 20000) {

....
BigInteger target = BigInteger.valueOf(Block.getBaseTarget()).multiply(BigInteger.valueOf(account.getEffectiveBalance())).multiply(BigInteger.valueOf(elapsedTime));
byte[] generationSignatureHash = MessageDigest.getInstance("SHA-256").digest(generationSignature);
BigInteger hit = new BigInteger(1, new byte[] {generationSignatureHash[7], generationSignatureHash[6], generationSignatureHash[5], generationSignatureHash[4], generationSignatureHash[3], generationSignatureHash[2], generationSignatureHash[1], generationSignatureHash[0]});
if (hit.compareTo(target) >= 0) {

return false;

}

} else {

....
}

return true;

} catch (Exception e) {

return false;

}

}

byte[] generationSignatureHash = MessageDigest.getInstance("SHA-256").digest(generationSignature); seems should be
byte[] generationSignatureHash = MessageDigest.getInstance("SHA-256").digest(previousBlock.generationSignature);
hero member
Activity: 784
Merit: 501
January 04, 2014, 11:51:04 PM
My observations about memory usage for keeping blockchain.
hero member
Activity: 784
Merit: 501
January 04, 2014, 10:10:17 PM
Can someone please point to the exact lines in the code where it is established that empty accounts cannot forge.
Block.verifyGenerationSignature(), line 1270:
Code:
                    if (account == null || account.getEffectiveBalance() == 0) {

                        return false;

                    }

But then...
Code:
https://localhost:7875/nxt?requestType=getBalance&account=1739068987193023818

Code:
{"balance":-99999798000,"effectiveBalance":-99999798000,"unconfirmedBalance":-99999798000}

So if genesis account is in memory cache, it can forge! I really donno, is it in cache or no.
Btw, seems like this check is not changed in 0.5.0.
hero member
Activity: 784
Merit: 501
January 04, 2014, 10:05:17 PM
Obviously: Before typing this post, I tried the latter method on my local 0.5.0 client and it didn't work. So I guess this has been fixed by now.
Yes, pushBlock() has check. From decompiled code:
Code:
            if(payloadLength > 32640 || 224 + payloadLength != buffer.capacity())
                return false;
full member
Activity: 224
Merit: 100
January 04, 2014, 08:33:45 PM
I'll just wait until you guys fix their broken code for them and then i'll fork it.

This stuff needs a ton of refactoring.

Any serious fork would not be recognizable from the original.

Anyway,  I recommend everyone to stay away from this coin until they can get their act together.

So much anger...

Keep on topic, please!
newbie
Activity: 28
Merit: 0
January 04, 2014, 07:56:47 PM
Have you ever heard of bug tracking software? They have only been around for like 20 years.
Why don't you google Bugzilla.
legendary
Activity: 868
Merit: 1000
Cryptotalk.org - Get paid for every post!
January 04, 2014, 07:43:10 PM
I'll just wait until you guys fix their broken code for them and then i'll fork it.

This stuff needs a ton of refactoring.

Any serious fork would not be recognizable from the original.

Anyway,  I recommend everyone to stay away from this coin until they can get their act together.
newbie
Activity: 56
Merit: 0
January 04, 2014, 07:25:53 PM
Uh, I seem to be stumbling over these things a lot today... another bug, this time just to take down the network. :p

let's look at this line of code from pushBlock:
block.transactions = new long[block.numberOfTransactions];

Looks pretty innocent, doesn't it? What could possibly go wrong?

Well, except, there's one thing: block.numberOfTransactions is an int, so with the right number, we could try to allocate 2G*8 = 16GB of RAM. This will cause an OutOfMemoryError from the JVM, which we can't catch with any try {} catch(Exception).

The line is getting a bit less innocent, but still, all the block stuff is checked befor we get here, right? RIGHT?...

Let's see, what is checked at that point by slowly going backwards...
At first we see a verifyBlockSignature and verifyGenerationSignature, checking, ehm, well, the blockSignature, generationSignature (that has to match the generatorPublicKey), and the hit.
Next, we see some even more trivial things: lastBlock is checked, and the current blockId has to be unknown.
Now we already arrive at the constructor. Surely, the numberOfTransactions has been checked for sanity before we construct the block... let's see.
Going further, we see checks for payloadLength and blockTimestamp, another previousBlockId check, and a version check.
Suddenly, we're at the beginning of pushBlock... Still there has been no check of the value of numberOfTransactions! It's getting quite not so innocent now... we now have to check all call points of pushBlock and whether buffer is guaranteed to be sane...

Let's start with the occurence in doPost() -> processBlock (It's the shortest, maybe we're lucky Smiley)
The buffer that goes to pushBlock is created by a block.getBytes() call, which basically is just a serialization (btw: y u no serialize???).
The block is created a few lines above using a variable called numberOfTransactions.
And this variable is read... *drum roll* ...from the post request that any person can send to the server.
Uh oh. Not innocent, at all...
So what do we have to do to take down the network?
It has to be our turn to generate a block - check
We have to generate it - check
and right before we calculate the signatures, we set the numberOfTransactions to INT_MAX - check (It's a one-line change in generateBlock, e.g. right before payloadHash is calculated)

So that was pretty easy to get all the clients to crash with an OutOfMemoryError.
But what to do, if we don't want to wait for our next turn to forge a block to take down the network?
There's an even easier solution Smiley
Let's take a look at "processBlock" again. It reads a few parameters of the block, generates the block, and ... *queue the drum roll again!* ... allocates a byte buffer of BLOCK_HEADER_LENGTH + payloadLength, without checking anything anywhere. So just post an INT_MAX payloadLength and garbage for the other params, and you've probably taken down a node because it wanted to allocate 2GB of RAM and I guess none of them has stuff like -Xmx=4096M or sth. set.
Phew... that was just too easy.

Obviously: Before typing this post, I tried the latter method on my local 0.5.0 client and it didn't work. So I guess this has been fixed by now.


[edit]
The same can be achieved by modifying what your node responds to a getNextBlocks call. Exactly the same procedure as above.
full member
Activity: 126
Merit: 100
Girls dont crypto?
January 04, 2014, 06:54:08 PM
I'll just wait until you guys fix their broken code for them and then i'll fork it.
legendary
Activity: 868
Merit: 1000
Cryptotalk.org - Get paid for every post!
January 04, 2014, 06:34:56 PM
How are the balances of an Account determined?
newbie
Activity: 56
Merit: 0
January 04, 2014, 05:46:54 PM
Where is the exact difference between balance and effective balance?
When a block gets analyzed the balance of the block-creater, and the balance of the recipients and senders of all transactions get increased/decreased.

Why do we need effective balance? The getEffectiveBalance method says that the effective balance is balance minus the amount the user got in the last block. Why does this amount get subtracted?

Yhe effectiveBalance is used to determine the "forging-power" an account has.
If that wasn't there, you could make sure that you'll always be the one to forge the next block.
With how it is now, you can only be quite certain, that you'll forge every block, see my bug report a few pages earlier. Wink

I see, but why does the amount that I possibly received in the last forged block gets subtracted?
//EDIT: Does that just mean, that I can use an amount that I got sent for the forging-process of the after next block and not for the next block?

Correct. Any amount received only counts for forging after the next block has been forged.
newbie
Activity: 50
Merit: 0
January 04, 2014, 05:19:20 PM
Where is the exact difference between balance and effective balance?
When a block gets analyzed the balance of the block-creater, and the balance of the recipients and senders of all transactions get increased/decreased.

Why do we need effective balance? The getEffectiveBalance method says that the effective balance is balance minus the amount the user got in the last block. Why does this amount get subtracted?

Yhe effectiveBalance is used to determine the "forging-power" an account has.
If that wasn't there, you could make sure that you'll always be the one to forge the next block.
With how it is now, you can only be quite certain, that you'll forge every block, see my bug report a few pages earlier. Wink

I see, but why does the amount that I possibly received in the last forged block gets subtracted?
//EDIT: Does that just mean, that I can use an amount that I got sent for the forging-process of the after next block and not for the next block?
newbie
Activity: 56
Merit: 0
January 04, 2014, 05:17:19 PM
Where is the exact difference between balance and effective balance?
When a block gets analyzed the balance of the block-creater, and the balance of the recipients and senders of all transactions get increased/decreased.

Why do we need effective balance? The getEffectiveBalance method says that the effective balance is balance minus the amount the user got in the last block. Why does this amount get subtracted?

Yhe effectiveBalance is used to determine the "forging-power" an account has.
If that wasn't there, you could make sure that you'll always be the one to forge the next block.
With how it is now, you can only be quite certain, that you'll forge every block, see my bug report a few pages earlier. Wink
newbie
Activity: 50
Merit: 0
January 04, 2014, 04:52:26 PM
Where is the exact difference between balance and effective balance?
When a block gets analyzed the balance of the block-creater, and the balance of the recipients and senders of all transactions get increased/decreased.

Why do we need effective balance? The getEffectiveBalance method says that the effective balance is balance minus the amount the user got in the last block. Why does this amount get subtracted?
newbie
Activity: 50
Merit: 0
January 04, 2014, 03:48:29 PM
Reminds me of finding the three flaws  Grin

legendary
Activity: 868
Merit: 1000
Cryptotalk.org - Get paid for every post!
January 04, 2014, 03:43:31 PM
Ok people... take a break from all the advanced thoughts... and help us coding-challenged people for a sec.!    Cheesy


Can someone please point to the exact lines in the code where it is established that empty accounts cannot forge.

Is the Genesis account excluded from forging?  What is the forging conditions set for an account with negative balance?


Just wondering what happens if someone unlocks the Genesis account... in respect to the code under examination, of course.


thnx   Wink

 Shocked if someone unllocks genesis account, could they send negative NXT to everone to reduce everyone's balance to zero?

I believe peers reject transactions from accounts with Negative balances.  CfB (or the source) can give the definitive on this.

Fingers crossed on this one!

Oh.... $#(#%&*!
legendary
Activity: 1232
Merit: 1001
January 04, 2014, 03:15:21 PM
Ok people... take a break from all the advanced thoughts... and help us coding-challenged people for a sec.!    Cheesy


Can someone please point to the exact lines in the code where it is established that empty accounts cannot forge.

Is the Genesis account excluded from forging?  What is the forging conditions set for an account with negative balance?


Just wondering what happens if someone unlocks the Genesis account... in respect to the code under examination, of course.


thnx   Wink

 Shocked if someone unllocks genesis account, could they send negative NXT to everone to reduce everyone's balance to zero?

I believe peers reject transactions from accounts with Negative balances.  CfB (or the source) can give the definitive on this.
full member
Activity: 126
Merit: 100
January 04, 2014, 03:11:02 PM
Ok people... take a break from all the advanced thoughts... and help us coding-challenged people for a sec.!    Cheesy


Can someone please point to the exact lines in the code where it is established that empty accounts cannot forge.

Is the Genesis account excluded from forging?  What is the forging conditions set for an account with negative balance?


Just wondering what happens if someone unlocks the Genesis account... in respect to the code under examination, of course.


thnx   Wink

 Shocked if someone unllocks genesis account, could they send negative NXT to everone to reduce everyone's balance to zero?

No, the transaction amount as well as the sender's balance have to be positive. Neither is the case in your apocalyptic scenario. Wink

That would be fun to try on a testnet though. Smiley
Pages:
Jump to: