Nxt source code flaw reports

[ricot]

exactly...

Btw: I still have quite a bit of code ahead of me.

[Come-from-Beyond]

Is there a minimum amount of transactions/recipient accounts required to generate the Genesis Block in order to establish a secure 100% POS currency? Could all 1 Billion NXT have gone to one recipient account; thus having one transaction in the Genesis Block? How would things progress from there in regard to acceptance of chains, signing, etc. in such case? Would it just progress the same way as it did with the existing Genesis Block?

thnx   

If u give 51% of all coins to one entity and don't have TF enabled then this entity can do 51% attack at any moment.

[bitcoinpaul]

TF to the rescue!!!

[BitByteBite]

Interesting callenge Ill take a closer look to the source code later. Has someone found a flaw yet or are they sill undiscoverd?

2 flaws were found. The fatal flaw with 100K reward is still waiting for u.

Sounds interesting

[msin]

CFB, would be cool if you could update the OP, just so people who come here know that the first two flaws have been found and the fatal flaw has yet to be found.

[BitByteBite]

CFB, would be cool if you could update the OP, just so people who come here know that the first two flaws have been found and the fatal flaw has yet to be found.

This is true, but now you have to find the posts that found the flaw to find the answer. A relatively small rabbit hole if you ask me

[bitcoinpaul]

This silence is calming

[nervermind]

6098:                String recipientValue = req.getParameter("recipient"), amountValue =  req.getParameter("amount"), feeValue = req.getParameter("fee"), deadlineValue = req.getParameter("deadline");

6109:             deadline = (short)(Double.parseDouble(deadlineValue) * 60);


deadlineValue comes from HttpServletRequest and passed to parseDouble which is vulnerable to DOS prior to JRE 6.23

see http://www.oracle.com/technetwork/topics/security/alert-cve-2010-4476-305811.html

[Come-from-Beyond]

6098:                String recipientValue = req.getParameter("recipient"), amountValue =  req.getParameter("amount"), feeValue = req.getParameter("fee"), deadlineValue = req.getParameter("deadline");

6109:             deadline = (short)(Double.parseDouble(deadlineValue) * 60);


deadlineValue comes from HttpServletRequest and passed to parseDouble which is vulnerable to DOS prior to JRE 6.23

see http://www.oracle.com/technetwork/topics/security/alert-cve-2010-4476-305811.html

Interesting catch! Fortunatelly, NRS doesn't work on Java 6.

[nervermind]

805:   static void loadBlocks(String fileName) throws Exception {
         
         FileInputStream fileInputStream = new FileInputStream(fileName);
         ObjectInputStream objectInputStream = new ObjectInputStream(fileInputStream);
         blockCounter = objectInputStream.readInt();
         blocks = (HashMap<Long, Block>)objectInputStream.readObject();
         lastBlock = objectInputStream.readLong();
         objectInputStream.close();
         fileInputStream.close();
         
      }
Streams are about to close at the end of the method. But if there is an Exception thrown (any of .read* can do that) - they are not.
Unreleased resources in servlets => DOS.

I've found few more such things if you're interested.

[Come-from-Beyond]

805:   static void loadBlocks(String fileName) throws Exception {
         
         FileInputStream fileInputStream = new FileInputStream(fileName);
         ObjectInputStream objectInputStream = new ObjectInputStream(fileInputStream);
         blockCounter = objectInputStream.readInt();
         blocks = (HashMap<Long, Block>)objectInputStream.readObject();
         lastBlock = objectInputStream.readLong();
         objectInputStream.close();
         fileInputStream.close();
         
      }
Streams are about to close at the end of the method. But if there is an Exception thrown (any of .read* can do that) - they are not.
Unreleased resources in servlets => DOS.

I've found few more such things if you're interested.

Yes, plz.

[nervermind]

OK. Here we go:

1209:   ObjectOutputStream objectOutputStream = new ObjectOutputStream(fileOutputStream);
same story here. If Exception is thrown - stream stays unreleased.

2877:    OutputStream outputStream = connection.getOutputStream();
there is a catch block at 2919, maybe close it there?

2883:          InputStream inputStream = connection.getInputStream();
same as above

3299:    static void loadTransactions(String fileName) throws Exception {
FileInputStream fileInputStream = new FileInputStream(fileName);
         ObjectInputStream objectInputStream = new ObjectInputStream(fileInputStream);

and

3437: saveTransactions

[Schollepetzer]

Hey boyz,

could someone give a short summary plz. about the 2 flaws already been found?
Not in the mood right now the read through 55p...  

Thank you

[FrictionlessCoin]

Seeking Additional Java Developers

We have successfully decompiled the Nxt source and are now in the process of performing a massive refactoring and cleanup of the code base.

Short term goals.

Remove the primitive addressing scheme and use a scheme that looks like Bitcoin.  Bitcoin addresses are more readable, have a checksum and have many tools to generate them.  Nxt addresses that are all numerical are something that came out of the 1950's.

Secure Wallet.  Nxt wallet is fundamentally insecure.  Exposing a wallet to the public internet for everyone to hack is just idiotic.   No wonder so many Nxt users have lost money!

Nxt does not use an internal database like every other alt coin.  This is idiotic because NXT nodes easily fail and run out of memory.  Furthermore, if you accidentally turn off your node... your wallet can get corrupted!

NEX strives not only to be a fairer distributed Nxt variant,  but a technologically super version.

Join the enterprise... participate with your coding skills!!!

We will not ship SHIT like the Nxt folks.  We will ship product that will secure our users coins!

May I join?

Time to defect to the winning team folks!

[Come-from-Beyond]

Time to defect to the winning team folks!

1st we must find the 3rd flaw... How will we split 100k btw?

[nervermind]

Does security flaw count?

This is a good example of Often Misused: Authentication (see https://www.owasp.org/index.php/Often_Misused:_Authentication)
5995: if (allowedUserHosts == null && !InetAddress.getByName(req.getRemoteAddr()).isLoopbackAddress()) {
6030: if (allowedUserHosts == null && !InetAddress.getByName(req.getRemoteAddr()).isLoopbackAddress()) {
6065: if (allowedUserHosts == null && !InetAddress.getByName(req.getRemoteAddr()).isLoopbackAddress()) {

This is a bad idea to rely on remoteAddr (which can spoofed) when you do authorization checks.
I didn't do any risk assessment of removePeer, removeActivePeer and removeBlacklistedPeer so have no idea how critical are these functions. But it looks like the idea was to restrict this functionality only to requests coming from localhost. And the way it is done... Bit insecure I'd say.

[Come-from-Beyond]

Does security flaw count?

Only if injected ones.

[liyi3c]

Code:

for (Peer peer : groupedPeers) {
							
							if (peer.date == validDate) {
								
								totalWeight += peer.weight;
								
							} else {
								
								peer.adjustedWeight = 0;
								peer.updateWeight_UserBroadCast();
								
							}
							
						}
						
						for (Peer peer : groupedPeers) {
							
							peer.adjustedWeight = 1000000000L * peer.weight / totalWeight;
							peer.updateWeight_UserBroadCast();
							
						}

the second 'for' loop assgin adjustedWeight to peer whose peer.date != validDate.
if one account attached to huge amount hallmarks, and send out them. as more and more its hallmarks out of date(peer.date != validDate), we should set their adjustedWeight = 0; but here it's not this way. the sum(peer.weight)/totalWeight will not equal to 1.
someone can increase its chosen possibility by send as much as hallmarks

[Come-from-Beyond]

the second 'for' loop assgin adjustedWeight to peer whose peer.date != validDate.
if one account attached to huge amount hallmarks, and send out them. as more and more its hallmarks out of date(peer.date != validDate), we should set their adjustedWeight = 0; but here it's not this way. the sum(peer.weight)/totalWeight will not equal to 1.
someone can increase its chosen possibility by send as much as hallmarks

I don't see a bug there...

[nervermind]

4788:      if (allowedBotHosts != null && !allowedBotHosts.contains(req.getRemoteHost())) {
6700:       if (allowedUserHosts != null && !allowedUserHosts.contains(req.getRemoteHost())) {


I wouldn't rely on getRemoteHost, in some cases it's not providing legit info. It can be spoofed.
Or it can be proxy/balancer address.