Nxt source code flaw reports

[gimre]

Btw, this has just come to my mind. Noone did an audit of Crypto and Curve25519 code yet. We would pay a reward for found flaws.

The comments in the code
   /* Ported  .............. 23/02/08.
    * Original: http://cds.xs4all.nl:8081/ecdh/
    */
   /* Generic 64-bit integer implementation of Curve25519 ECDH
    * ..... 200608242056
    * Public domain.
    *

cause such impression that the code is old enough and likely reviewed enough, i.e. cleared from errors.

Wouldn't assume that.
keygen (in the form of core(P, null, k, null)) and computations themselves have probably been verified - they wouldn't work otherwise,
but I wouldn't assume that regarding sign + verify part.

[1714238681]

1714238681

[1714238681]

1714238681

[1714238681]

1714238681

[1714238681]

1714238681

[CrazyEyes]

There are several versions out there. Would not assume until further notice.

Edit: At a first glance, the google code version seems legit though.

[gimre]

Edit: At a first glance, the google code version seems legit though.

I've only seen copies, and copies of a copies.
Have you found any actual analysis of signing + verify part?
(I'm referring to Curve.sign and Curve.verify not to it's Crypto.* wrappers)

[CrazyEyes]

Edit: At a first glance, the google code version seems legit though.

I've only seen copies, and copies of a copies.
Have you found any actual analysis of signing + verify part?
(I'm referring to Curve.sign and Curve.verify not to it's Crypto.* wrappers)

No, i have not. Only equations of the algorithm. Trying to understand if Curve.sign|verify does what its supposed to. Join the irc channel and we can talk further.. if you want. There are some good guys in there, ZjP is one of them.

Regards
j0b

[BloodyRookie]

This is close to one of the injected flaws, so I can't give u more details. If u think it's a flaw u r supposed to describe it, not ask questions.

The Crypto  class implementation doesn't check the return value of sign(...). It should because it will detect if the signature is 0.

No. But this situation is avoided by https://bitbucket.org/JeanLucPicard/nxt-public/src/4073c21098076d3469b3f74d49e73ffabe3a2001/Nxt.java?at=master#cl-3458

But only concerning Transactions. When an account generates a block, there are no retries if verifying the signed block fails:

Code:

		block.blockSignature = Nxt.Crypto.sign(data2, secretPhrase);
		JSONObject request = block.getJSONObject(newTransactions);
		request.put("requestType", "processBlock");
		if ((block.verifyBlockSignature()) && (block.verifyGenerationSignature())) {
			Nxt.Peer.sendToAllPeers(request);
		} else {
			Nxt.logMessage("Generated an incorrect block. Waiting for the next one...");
		}

People are getting the message "Generated an incorrect block. Waiting for the next one..." sometimes.

[gimre]

Good. We'll get some bitcoins to set a reward for Nxt crypto algo audit. Yes, bitcoins, not nxts, coz if the algo is flawed nxts won't be worth a lot .

@CfB, have the developers considered switching to Ed25519 instead?
I think it could easily be deployed (i.e, require all txes + blocks > 40k block to have new sig).

There are many advantages.
First one being it's much better verified and tested than current sing+verify.
Second one is probably speed (although I haven't tested), as computations in Edward's form should be easier.

[gimre]

No, i have not. Only equations of the algorithm. Trying to understand if Curve.sign|verify does what its supposed to. Join the irc channel and we can talk further.. if you want. There are some good guys in there, ZjP is one of them.

Will, try, but I find IRC distracting from doing STUFF .
Also, after looking briefly equations looked okay, what I'm more concerned about is the code,
that in different places sometimes uses so called "reduced form" and sometimes not...

[gimre]

You just need to cause a collission of the block ID, which is 64bit.

Critical flaw description:

Code:

Only 64 bits of the previous block hash are used. This gives ability to inject blocks with another set of transactions.

SHA256-hash:

Code:

888f278c773d39b8334a651d84ee78871bd0e5d45e09be8fdb190ba1b2969530

Looks legit. Where to send 10K to?

Hmm, wasn't it the same way in 0.4.7 (and even further ones)?
If so, how is it "injected flaw" :>

[BloodyRookie]

No, i have not. Only equations of the algorithm. Trying to understand if Curve.sign|verify does what its supposed to. Join the irc channel and we can talk further.. if you want. There are some good guys in there, ZjP is one of them.
Regards
j0b

Can you please ask that guy what the reasoning behind the strange is_negative(long10 x) function is? It's kind of hard to understand.

[jl777]

Good. We'll get some bitcoins to set a reward for Nxt crypto algo audit. Yes, bitcoins, not nxts, coz if the algo is flawed nxts won't be worth a lot .

@CfB, have the developers considered switching to Ed25519 instead?
I think it could easily be deployed (i.e, require all txes + blocks > 40k block to have new sig).

There are many advantages.
First one being it's much better verified and tested than current sing+verify.
Second one is probably speed (although I haven't tested), as computations in Edward's form should be easier.

I am pretty sure BCNext decided to use current approach over Ed25519. Can't remember the exact reason, maybe it was to be able to run on cellphones?

James

[CrazyEyes]

No, i have not. Only equations of the algorithm. Trying to understand if Curve.sign|verify does what its supposed to. Join the irc channel and we can talk further.. if you want. There are some good guys in there, ZjP is one of them.

Will, try, but I find IRC distracting from doing STUFF .
Also, after looking briefly equations looked okay, what I'm more concerned about is the code,
that in different places sometimes uses so called "reduced form" and sometimes not...

Found several of those as well. Hehe allright, well im in there, so now you know:D Good point though. Maybe we, the community, should stop error search NXT source code (even it is so damn interesting) and start benefit our selfs?:p This is like code review for free Even if you find some questionable code which turns out, can be better accomplished, is still go by unnoticed. And this guy, 100K, which will be happy (lotto?). Then people would stop bug searching. Do not know how much time i spend on shitz for next. hehe. for free.

Humble regards
j0b

[ricot]

Ok, some Crypto/Curve things that I just randomly stumbled across:
As someone else already mentioned: Crypto.sign could use the boolean returned by Curve25519.sign instead of going all the way to verify it before regenerating the signature. Would save some CPU cycles.
generateAuthorizationToken only signs, without checking, i.e. they are sometimes invalid. That's nothing serious, just an annoyance for people using it. As for the endless if-else: String.format is your friend.
Another thing to take care about with generateAuthorization token is to actually check the blockchain, whether the public key that was transamitted was used for that account, otherwise you only get 64bit security, but anyways, that's for clients implementing the system, I just think it should be mentioned somewhere.
In unlockAccount it will display a deadline eventhough it may already know that the signature is invalid. If you don't do that and also add a check when generating the block, you could get rid of the "invalid block generated" message that some people are seeing from time to time and get upset about.
If you say that approx. 1/4 of sign operations fail, that means both, the generation signature and the block signature can fail at 1/4 each, so actually only 56.25 % of all block generations are successful. (really?) Evil Bob could increase his chances of generating a block to 75% if he detects a wrong block signature and calculates it again, leaving out a transaction or doing some other harmless stuff to change the data.
And, as I've written before, you're using the shortcut of h = hash(m, Y) and transmitting h, which as far as I know has never been proven to be secure.

[edit]
Btw: It seems that no-one has sent a message-transaction yet... So no funny forking action in the client... Much too boring out there...

[Come-from-Beyond]

This is close to one of the injected flaws, so I can't give u more details. If u think it's a flaw u r supposed to describe it, not ask questions.

The Crypto  class implementation doesn't check the return value of sign(...). It should because it will detect if the signature is 0.

No. But this situation is avoided by https://bitbucket.org/JeanLucPicard/nxt-public/src/4073c21098076d3469b3f74d49e73ffabe3a2001/Nxt.java?at=master#cl-3458

But only concerning Transactions. When an account generates a block, there are no retries if verifying the signed block fails:

Code:

		block.blockSignature = Nxt.Crypto.sign(data2, secretPhrase);
		JSONObject request = block.getJSONObject(newTransactions);
		request.put("requestType", "processBlock");
		if ((block.verifyBlockSignature()) && (block.verifyGenerationSignature())) {
			Nxt.Peer.sendToAllPeers(request);
		} else {
			Nxt.logMessage("Generated an incorrect block. Waiting for the next one...");
		}

People are getting the message "Generated an incorrect block. Waiting for the next one..." sometimes.

This is very close... R u ready to describe the flaw?

[Come-from-Beyond]

Good. We'll get some bitcoins to set a reward for Nxt crypto algo audit. Yes, bitcoins, not nxts, coz if the algo is flawed nxts won't be worth a lot .

@CfB, have the developers considered switching to Ed25519 instead?
I think it could easily be deployed (i.e, require all txes + blocks > 40k block to have new sig).

There are many advantages.
First one being it's much better verified and tested than current sing+verify.
Second one is probably speed (although I haven't tested), as computations in Edward's form should be easier.

Do u have fast java implementation?

[Come-from-Beyond]

Hmm, wasn't it the same way in 0.4.7 (and even further ones)?

It's fixed by adding previousBlockHash.

[Come-from-Beyond]

Evil Bob could increase his chances of generating a block to 75% if he detects a wrong block signature and calculates it again, leaving out a transaction or doing some other harmless stuff to change the data.

Could u explain this?

[ricot]

When generating a block, you have 2 signatures that could go wrong.
One is the generationSignature, which is calculated from the previousBlock, so there is no chance in changing it when it fails. (i.e. 25% invalid block).
The other one is the blockSignature. Currently, this is only generated once and contains a hash of e.g. all the transactions. If that signature fails in NRS, the client will just output invalid block and don't continue to try.
If Bob isn't using NRS, he could generate the blockSignature again with different data, i.e. different transactions. It's relatively easy to find a payload that will generate a valid hash.
Bob could then transmit the perfectly fine block, which NRS would have not enabled him to do. So he got an advantage by not using the default client.

[ricot]

And another thing about the difficulty:
I think that thing is much too "flaky".
Based on a single random event (someone generating a block at a certain time), the difficulty can get a value of between 0.5-2 times its old value. So if a few people, by chance, generate a few blocks in fast succession, the value will be rediculusly low and the next block will take ages to be generated. (Btw: that just happened, we had a 24 minute block, and now it's going to be followed by a 9+ minutes block).
If the difficulty would change more smooth, the underlying randomness would also smooth out and you would get a much more even distribution of blocks over time.

[Come-from-Beyond]

When generating a block, you have 2 signatures that could go wrong.
One is the generationSignature, which is calculated from the previousBlock, so there is no chance in changing it when it fails. (i.e. 25% invalid block).
The other one is the blockSignature. Currently, this is only generated once and contains a hash of e.g. all the transactions. If that signature fails in NRS, the client will just output invalid block and don't continue to try.
If Bob isn't using NRS, he could generate the blockSignature again with different data, i.e. different transactions. It's relatively easy to find a payload that will generate a valid hash.
Bob could then transmit the perfectly fine block, which NRS would have not enabled him to do. So he got an advantage by not using the default client.

Well, u and BloodyRookie explained 99% of the serious flaw. I think u both should get 1'000 NXT. Post ur account.

@BloodyRookie: Post ur Nxt account, plz.


PS: The description of the serious flaw is:

Code:

An incorrect block signature is not recalculated. This gives ability to use less than 50% of the stake to attack the network.

[ricot]

Thanks! And congrats to BloodyRookie

Account is the same as always...