Nxt source code flaw reports

[ricot]

You have to think like an attacker. They can do this by producing otherwise-valid blocks with high difficulties to knock out most of the network.

What do u mean?

I could be wrong, but it seems like some of the responses here assume a system of well-behaved participants. For threat-modeling you need to think like an attacker and how best to compromise the system.

For example, an attacker that wants to take over the NXT network could create a reasonably-looking block (e.g. it has a valid sender, recipient, password, etc.) and modify it slightly so that it difficulty is orders of magnitudes higher than the last block. All other nodes should accept his block because of the higher difficulty but nodes sending other blocks at that time would be blacklisted (for sending lower-complexity blocks). If the network was flooded with rouge agents sending high complexity blocks, I think it's possible for them to take over the network. And if they own the network they can do pretty much anything.

actually the difficulty thing doesn't work like that, you can't just generate a block with a high difficulty, because with the private keys you, as the attacker, have available, the block would have to have a timestamp far in the future and so will not be accepted by the peers. (i've already explained the little loophole in there that is caused by a slightly wrong implementation a few pages before here, look for anna gaining 7.5% more forges )

[iscy]

To fix this weakness forever, a simple no money, no fee transaction could be executed. Whenever you open a wallet, if the NXT client notices that your account on the chain doesn't have a public key associated with, it could issue a 0nxt/0fee transaction that is meant to record the key.

So every new account which gets opened just for a second would get 'loaded' into the blockchain. then, someone would open thousands of thousands of accounts to bloat the blockchain just for fun. i don't like.

wrong?

No, only when opening an account that has no public key associated to it that you can do this. If the account doesn't exist yet (has not received any transactions), then the operation should fail.

[klee]

Jaguar post your id (or pm)...

14544444716689899421 Thanks!

Received?

[kunibopl]

@klee: what you are doing for the community is superb.

I don't have such measures to support the development but I am working on a website, that allows all students and stuff of my university to load their CampusCard with NXT. The whole concept is not totally clear right now, because it depends on the alias-system.
but anyway it will include a local exchange €<->NXT.
In the future I hope there also will be anonymous paybacksystems, that allow users to exchange their paybackpoints to NXT.
I think cryptos are very suitable for anonymous paybacksystems and massadoption would be facilitated.

[Jaguar0625]

Jaguar post your id (or pm)...

14544444716689899421 Thanks!

Received?

Yep. I got it. Thanks a lot!

[okwolf]

Some of these are probably duplicates by now, but here's my 2 NXT:

• The entire source is implemented as a single file with lots of inner Classes. To enable distributed development this should be decomposed to at least one file per Class. This is especially true for Curve25519, which just added more clutter when merged into the same file.
• The genesis block is hardcoded into the source. Generally static data resources should be packaged separately as some sort of format that can be easily restored when requested.
• Why are hex strings used for serializing binary data to and from JSON over the wire? Other encodings such as Base64 would save on bandwidth and overhead of the protocol.
• What is the benefit of using unsigned longs for most of the ids? In particular the accounts. Although the hash is giving 256bits of output, 192 of them are being truncated and ignored in these cases.
• One barrier toward wider adoption of NXT is the requirement for whole numbers for amounts and fees. So far the code appears to have all of those values as ints or longs which would make transitioning to a divisible NXT difficult.

Lastly, have you guys thought about providing unit tests or using quality analysis tools like FindBugs, Checkstyle, or PMD? We're going through this at my day job right now, and I have to say early in the process is the best time to try set such standards.

[utopianfuture]

Some of these are probably duplicates by now, but here's my 2 NXT:

• The entire source is implemented as a single file with lots of inner Classes. To enable distributed development this should be decomposed to at least one file per Class. This is especially true for Curve25519, which just added more clutter when merged into the same file.
• The genesis block is hardcoded into the source. Generally static data resources should be packaged separately as some sort of format that can be easily restored when requested.
• Why are hex strings used for serializing binary data to and from JSON over the wire? Other encodings such as Base64 would save on bandwidth and overhead of the protocol.
• What is the benefit of using unsigned longs for most of the ids? In particular the accounts. Although the hash is giving 256bits of output, 192 of them are being truncated and ignored in these cases.
• One barrier toward wider adoption of NXT is the requirement for whole numbers for amounts and fees. So far the code appears to have all of those values as ints or longs which would make transitioning to a divisible NXT difficult.

Lastly, have you guys thought about providing unit tests or using quality analysis tools like FindBugs, Checkstyle, or PMD? We're going through this at my day job right now, and I have to say early in the process is the best time to try set such standards.

Hi are you new to crypto or you just created a new account here ?

[ImmortAlex]

Peer.analyzeHallmark():

Code:

if (host.length() > 100 || !host.equals(realHost)) {
    return false;
}

"markHost" request:

Code:

if (host.length() > 100) {
    response.put("errorCode", 4);
    response.put("errorDescription", "Incorrect \"host\" (the length exceeds 100 chars limit)");
}

"decodeHallmark" has same check too.

Why there's check on 100 chars long? Host (peer.announcedAddress, wellKnownHosts too) can be any hostname, why do you limit it?

[Come-from-Beyond]

Why there's check on 100 chars long? Host (peer.announcedAddress, wellKnownHosts too) can be any hostname, why do you limit it?

Just in case. 640 Kb 100 chars is enough.

[windjc]

To fix this weakness forever, a simple no money, no fee transaction could be executed. Whenever you open a wallet, if the NXT client notices that your account on the chain doesn't have a public key associated with, it could issue a 0nxt/0fee transaction that is meant to record the key.

So every new account which gets opened just for a second would get 'loaded' into the blockchain. then, someone would open thousands of thousands of accounts to bloat the blockchain just for fun. i don't like.

wrong?

I have no idea why this implementation did not start with Bitcoin ideas and improved on them.

This re-invent the wheel from scratch really isn't a good idea.

All one had to do was replace Bitcoin's proof of work with NXT proof of stake.  But no,  it was more like, let's building everything from the ground up.

Why are you on here bitching about everything? What is your angle?

You aren't helping fix the issues. But there are a gang of people who are. So the issues will be fixed. Meanwhile go find something productive to do with your life please.

[gsan1]

I have a question regarding referencedTransactions. ReferencedTransactions are used so that a transaction gets only confirmed when the other transaction is already confirmed.

Okay, lets say I am creating a transaction T2 which has T1 as referencedTransaction. T1 is in the latest block, so T2 gets confirmed in the next block. What if the block in which T1 is in a fork chain and gets orphaned later? T1 gets added to the unconfirmedTransaction list. Now it could be that the deadline is expired and the transaction gets lost. T2 is in the main chain and is confirmed. So the referencedTransaction scheme does not work in that case?

[Come-from-Beyond]

I have a question regarding referencedTransactions. ReferencedTransactions are used so that a transaction gets only confirmed when the other transaction is already confirmed.

Okay, lets say I am creating a transaction T2 which has T1 as referencedTransaction. T1 is in the latest block, so T2 gets confirmed in the next block. What if the block in which T1 is in a fork chain and gets orphaned later? T1 gets added to the unconfirmedTransaction list. Now it could be that the deadline is expired and the transaction gets lost. T2 is in the main chain and is confirmed. So the referencedTransaction scheme does not work in that case?

If the block with T1 is orphaned then the block with T2 is orphaned too.

[rlh]

I have a question regarding referencedTransactions. ReferencedTransactions are used so that a transaction gets only confirmed when the other transaction is already confirmed.

Okay, lets say I am creating a transaction T2 which has T1 as referencedTransaction. T1 is in the latest block, so T2 gets confirmed in the next block. What if the block in which T1 is in a fork chain and gets orphaned later? T1 gets added to the unconfirmedTransaction list. Now it could be that the deadline is expired and the transaction gets lost. T2 is in the main chain and is confirmed. So the referencedTransaction scheme does not work in that case?

If the block with T1 is orphaned then the block with T2 is orphaned too.

FYI, this is actually how Satoshi Dice sends semi-instant payouts.  They include your payment as part of the reward-- if you try to double-spend, their transaction back to you is invalidated, when your transaction is invalidated.  Likewised, if your TX ends up in an orphaned block, their payment will subsequently be part of an orphaned chain.

So... semi-instant transactions are possible under certain circumstances.

[Jaguar0625]

I'm pretty impressed that no one has found an injected flaw yet. Great job hiding them!

I was rereading the initial post and realized that I don't understand what's the point of the SHA256 hashes in it.

Nxt source code has been released - https://bitcointalk.org/index.php?topic=345619.msg4287127#msg4287127

The code contains 3 flaws - serious, critical and fatal. The 1st person who reports these flaws will get 1'000, 10'000 or 100'000 NXT reward accordingly.

Each flaw has a small description. Here r SHA256 hashes of these descriptions:

bd34c891e9e3df9ea8b8eafc4dc3edc129f81365d42bf204ea58271e320f3ce5 - 1K reward
888f278c773d39b8334a651d84ee78871bd0e5d45e09be8fdb190ba1b2969530 - 10K reward
f5236644f4306699bb0fa90a905afe2454683c0aad6995e4433d712e2fdb257c - 100K reward

[Come-from-Beyond]

I'm pretty impressed that no one has found an injected flaw yet. Great job hiding them!

I was rereading the initial post and realized that I don't understand what's the point of the SHA256 hashes in it.

Nxt source code has been released - https://bitcointalk.org/index.php?topic=345619.msg4287127#msg4287127

The code contains 3 flaws - serious, critical and fatal. The 1st person who reports these flaws will get 1'000, 10'000 or 100'000 NXT reward accordingly.

Each flaw has a small description. Here r SHA256 hashes of these descriptions:

bd34c891e9e3df9ea8b8eafc4dc3edc129f81365d42bf204ea58271e320f3ce5 - 1K reward
888f278c773d39b8334a651d84ee78871bd0e5d45e09be8fdb190ba1b2969530 - 10K reward
f5236644f4306699bb0fa90a905afe2454683c0aad6995e4433d712e2fdb257c - 100K reward

To prove that we don't cheat by saying "No, this is not the flaw that we injected".

[rlh]

I'm pretty impressed that no one has found an injected flaw yet. Great job hiding them!

I was rereading the initial post and realized that I don't understand what's the point of the SHA256 hashes in it.

Nxt source code has been released - https://bitcointalk.org/index.php?topic=345619.msg4287127#msg4287127

The code contains 3 flaws - serious, critical and fatal. The 1st person who reports these flaws will get 1'000, 10'000 or 100'000 NXT reward accordingly.

Each flaw has a small description. Here r SHA256 hashes of these descriptions:

bd34c891e9e3df9ea8b8eafc4dc3edc129f81365d42bf204ea58271e320f3ce5 - 1K reward
888f278c773d39b8334a651d84ee78871bd0e5d45e09be8fdb190ba1b2969530 - 10K reward
f5236644f4306699bb0fa90a905afe2454683c0aad6995e4433d712e2fdb257c - 100K reward

I think CnB wrote up a brief description of each flaw, and then hashed the text.  This way, he can verify that he, in fact, knew what the flaws were from the beginning.

This is like sending yourself a letter today, predicting what will happen in 2014.  You can validate your predictions in 2015, if you have an early-dated post mark on the letter.

[ImmortAlex]

I'm lurking through code that tries to sync blocks with other peer. I mean thread started at line ~4463 (I call it "loading thread").

It requests "getMilestoneBlockIds". Peer process this request with code

Code:

int jumpLength = block.height * 4 / 1461 + 1;
while (block.height > 0) {
    milestoneBlockIds.add(convert(block.getId()));
    for (int i = 0; i < jumpLength && block.height > 0; i++) {
        block = blocks.get(block.previousBlock);
    }
}

If I not fail with elementary math, this code returns about four blockIds for one day. So day by day this request moves more and more bytes through network.
What for?
Note: list is in descending order, first block in list is last block in blockchain.

Take a look back to loading thread code.

Code:

for (int i = 0; i < milestoneBlockIds.size(); i++) {
    long blockId = (new BigInteger((String)milestoneBlockIds.get(i))).longValue();
    Block block = blocks.get(blockId);
    if (block != null) {
        commonBlockId = blockId;
        break;
    }
}

This code executes only if peer has higher cumulative difficulty, so it's last block never match with our's. So even if we're not in forked chain, common block will be about six hour ago. Or about 360 blocks ago.

So next request to peer will be "getNextBlockIds", which submissivelly returns about 360 blockIds. Many bytes! If we're not in forked chain, chances are very high that only one or two last ids are interesting for us.

But what if we're forked too deep? Take a look at this check:

Code:

if (Block.getLastBlock().height - blocks.get(commonBlockId).height < 720)

We're not interested in very low common block anyway!

If my assumptions are correct (and I no failed in elementary math), it's better to:
1. Limit "getMilestoneBlockIds" request with sending lower height of common block.
2. Send 1st, 2nd, 4th, 8th and so on blockIds from the top down to lower height, in response to this request. Or use some other principle, but assume that latest blocks are more wanted most times.

Am I miss something?

[Come-from-Beyond]

Am I miss something?

The logic of this code is as follows:

1. We ask a peer for milestone blocks.
2. We find the most recent milestone block that we know about.
3. We ask next blocks after the milestone one, until we see an unknown block.
4. We download all next blocks.

Gap between milestone blocks is increasing each day, but we'll never need to ask for a lot of ids. Usualy we need only 1 request for milestone blocks and 1 more for next blocks.

[ImmortAlex]

Yes, I'm failed with elementary math

Code:

int jumpLength = block.height * 4 / 1461 + 1;

It returns 365 blockIds everytime, so when one year will pass, milestone ids will be one for each day.
So we have more or less the same response for "getMilestoneBlockIds", but longer and longer for each "getNextBlockIds".
Are all rest assumptions correct?
I see a lot of unnecessary traffic, every second.

[Come-from-Beyond]

Yes, I'm failed with elementary math

Code:

int jumpLength = block.height * 4 / 1461 + 1;

It returns 365 blockIds everytime, so when one year will pass, milestone ids will be one for each day.
So we have more or less the same response for "getMilestoneBlockIds", but longer and longer for each "getNextBlockIds".
Are all rest assumptions correct?
I see a lot of unnecessary traffic, every second.

If u catch blocks from genesis block then yes, more and more traffic will be required.