Bitcoin Forum
November 18, 2024, 05:56:08 AM *
News: Latest Bitcoin Core release: 28.0 [Torrent]
 
   Home   Help Search Login Register More  
Warning: One or more bitcointalk.org users have reported that they believe that the creator of this topic displays some red flags which make them high-risk. (Login to see the detailed trust ratings.) While the bitcointalk.org administration does not verify such claims, you should proceed with extreme caution.
Pages: « 1 2 [3] 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 ... 65 »
  Print  
Author Topic: Nxt source code flaw reports  (Read 113375 times)
xibeijan
Legendary
*
Offline Offline

Activity: 1232
Merit: 1001


View Profile
January 03, 2014, 02:13:54 PM
 #41

I don't understand this part

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

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

What do you mean by "small description"?

What is the input that gives these hashes?  The source code?

Notable projects 2019: Semux, Dero, Wagerr, BEAM
bitcoinpaul
Hero Member
*****
Offline Offline

Activity: 910
Merit: 1000



View Profile
January 03, 2014, 02:16:03 PM
 #42

The prewritten statements for the 3 flaws.
ImmortAlex
Hero Member
*****
Offline Offline

Activity: 784
Merit: 501


View Profile
January 03, 2014, 02:18:56 PM
 #43

Edit: I can't explain why, coz I can leak info about a flaw accidentally, sorry.
Let me guess, we need to add amount -= transaction.amount when transaction.senderPublicKey is equals to publicKey?
I mean loop in Account.getEffectiveBalance() method.

It can be wrong assumption because I still doesn't understand well what is effective balance Smiley
Jaguar0625
Sr. Member
****
Offline Offline

Activity: 299
Merit: 250


View Profile
January 03, 2014, 02:47:45 PM
 #44

Code:
							Transaction transaction = Nxt.transactions.remove(block.transactions[i]);
unconfirmedTransactions.put(block.transactions[i], transaction);

Account senderAccount = accounts.get(Account.getId(transaction.senderPublicKey));
synchronized (senderAccount) {

senderAccount.setBalance(senderAccount.balance + (transaction.amount + transaction.fee) * 100L);

}

Account recipientAccount = accounts.get(transaction.recipient);
synchronized (recipientAccount) {

recipientAccount.setBalance(recipientAccount.balance - transaction.amount * 100L);
recipientAccount.setUnconfirmedBalance(recipientAccount.unconfirmedBalance - transaction.amount * 100L);

}

JSONObject addedUnconfirmedTransaction = new JSONObject();
addedUnconfirmedTransaction.put("index", transaction.index);
addedUnconfirmedTransaction.put("timestamp", transaction.timestamp);
addedUnconfirmedTransaction.put("deadline", transaction.deadline);
addedUnconfirmedTransaction.put("recipient", convert(transaction.recipient));
addedUnconfirmedTransaction.put("amount", transaction.amount);
addedUnconfirmedTransaction.put("fee", transaction.fee);
addedUnconfirmedTransaction.put("sender", convert(Account.getId(transaction.senderPublicKey)));
addedUnconfirmedTransaction.put("id", convert(transaction.getId()));
addedUnconfirmedTransactions.add(addedUnconfirmedTransaction);

i think there might be a bug here. accounts is just a HashMap, so accounts.get can return null.

if the sender sender is invalid, you will just get a null reference exception before doing anything.

But, if the sender is valid and the recipient is not, you would have already updated the sender's balance but throw a null reference exception before updating the recipient's balance.

And if you ever get into this situation, it appears to be non-recoverable because you will never be able to pop the last block. The code will always throw before getting to:

Code:
lastBlock = block.previousBlock;

And the popLastBlock function will return false exiting the loops you're calling it from:
Code:
while (lastBlock != commonBlockId && Block.popLastBlock())

NEM - nem.io
rlh
Hero Member
*****
Offline Offline

Activity: 804
Merit: 1004


View Profile
January 03, 2014, 03:02:57 PM
 #45

I'm not a BTC hater, but a small part of me hopes that the 100k flaw is an injection of a technique that is part of the bitcoin, and all other altcoin, protocols.

Let's just say it would be a passive way of proving you've done your crypto-currency homework.

Kudos to this event.  I hope this drums up a lot more support and attention for this innovative coin.

Now if we can just get someone to write this in C...

A Personal Quote on BTT from 2011:
"I'd be willing to make a moderate "investment" if the value of the BTC went below $2.00.  Otherwise I'll just have to live with my 5 BTC and be happy. :/"  ...sigh.  If only I knew.
Come-from-Beyond (OP)
Legendary
*
Offline Offline

Activity: 2142
Merit: 1010

Newbie


View Profile
January 03, 2014, 03:34:36 PM
 #46

Edit: I can't explain why, coz I can leak info about a flaw accidentally, sorry.
Let me guess, we need to add amount -= transaction.amount when transaction.senderPublicKey is equals to publicKey?
I mean loop in Account.getEffectiveBalance() method.

It can be wrong assumption because I still doesn't understand well what is effective balance Smiley

It's an incorrect guess. The flaws r more sophisticated.
Come-from-Beyond (OP)
Legendary
*
Offline Offline

Activity: 2142
Merit: 1010

Newbie


View Profile
January 03, 2014, 03:36:26 PM
 #47

Code:
							Transaction transaction = Nxt.transactions.remove(block.transactions[i]);
unconfirmedTransactions.put(block.transactions[i], transaction);

Account senderAccount = accounts.get(Account.getId(transaction.senderPublicKey));
synchronized (senderAccount) {

senderAccount.setBalance(senderAccount.balance + (transaction.amount + transaction.fee) * 100L);

}

Account recipientAccount = accounts.get(transaction.recipient);
synchronized (recipientAccount) {

recipientAccount.setBalance(recipientAccount.balance - transaction.amount * 100L);
recipientAccount.setUnconfirmedBalance(recipientAccount.unconfirmedBalance - transaction.amount * 100L);

}

JSONObject addedUnconfirmedTransaction = new JSONObject();
addedUnconfirmedTransaction.put("index", transaction.index);
addedUnconfirmedTransaction.put("timestamp", transaction.timestamp);
addedUnconfirmedTransaction.put("deadline", transaction.deadline);
addedUnconfirmedTransaction.put("recipient", convert(transaction.recipient));
addedUnconfirmedTransaction.put("amount", transaction.amount);
addedUnconfirmedTransaction.put("fee", transaction.fee);
addedUnconfirmedTransaction.put("sender", convert(Account.getId(transaction.senderPublicKey)));
addedUnconfirmedTransaction.put("id", convert(transaction.getId()));
addedUnconfirmedTransactions.add(addedUnconfirmedTransaction);

i think there might be a bug here. accounts is just a HashMap, so accounts.get can return null.

if the sender sender is invalid, you will just get a null reference exception before doing anything.

But, if the sender is valid and the recipient is not, you would have already updated the sender's balance but throw a null reference exception before updating the recipient's balance.

And if you ever get into this situation, it appears to be non-recoverable because you will never be able to pop the last block. The code will always throw before getting to:

Code:
lastBlock = block.previousBlock;

And the popLastBlock function will return false exiting the loops you're calling it from:
Code:
while (lastBlock != commonBlockId && Block.popLastBlock())

I'll check if this situation is possible but this is not one of the injected flaws.
lr127
Newbie
*
Offline Offline

Activity: 35
Merit: 0


View Profile
January 03, 2014, 04:51:24 PM
 #48

Come-from-Beyond
Where is aliases? In getTransaction as example.
In version NRS 0.4.7 they exists.
Come-from-Beyond (OP)
Legendary
*
Offline Offline

Activity: 2142
Merit: 1010

Newbie


View Profile
January 03, 2014, 05:00:40 PM
 #49

Come-from-Beyond
Where is aliases? In getTransaction as example.
In version NRS 0.4.7 they exists.

Alias System is an advanced feature and was removed from the source code.
BloodyRookie
Hero Member
*****
Offline Offline

Activity: 687
Merit: 500


View Profile
January 03, 2014, 05:39:28 PM
 #50

Jean-Luc stated:

Quote
This is the source of the Nxt Reference Software (NRS) corresponding to version 0.4.7e, as provided by its original developer, BCNext.

Does that mean, the flaws were incorporated into 0.4.7e which was distributed to the nxt users?

Nothing Else Matters
NEM: NALICE-LGU3IV-Y4DPJK-HYLSSV-YFFWYS-5QPLYE-ZDJJ
NXT: 11095639652683007953
Come-from-Beyond (OP)
Legendary
*
Offline Offline

Activity: 2142
Merit: 1010

Newbie


View Profile
January 03, 2014, 05:44:06 PM
 #51

Jean-Luc stated:

Quote
This is the source of the Nxt Reference Software (NRS) corresponding to version 0.4.7e, as provided by its original developer, BCNext.

Does that mean, the flaws were incorporated into 0.4.7e which was distributed to the nxt users?

It means that original 0.4.7e source code was modified before publishing.

Edit: U can try to find differences between 0.4.7e binaries and 0.4.7e source code, but u still must explain found flaws to get bounties.
rlh
Hero Member
*****
Offline Offline

Activity: 804
Merit: 1004


View Profile
January 03, 2014, 06:01:11 PM
 #52

Ok, to be sure.  If I compile this code and run it, I should be able to download and process the blockchain up to block 30k?  It would be nice is there was a testnet for just for the source-- that way we could test-process transactions.

A Personal Quote on BTT from 2011:
"I'd be willing to make a moderate "investment" if the value of the BTC went below $2.00.  Otherwise I'll just have to live with my 5 BTC and be happy. :/"  ...sigh.  If only I knew.
Come-from-Beyond (OP)
Legendary
*
Offline Offline

Activity: 2142
Merit: 1010

Newbie


View Profile
January 03, 2014, 06:05:02 PM
 #53

Ok, to be sure.  If I compile this code and run it, I should be able to download and process the blockchain up to block 30k?  It would be nice is there was a testnet for just for the source-- that way we could test-process transactions.

U'll be able to catch blocks till #22000. U would get all 30000 blocks but BCNext removed Alias System so ur version will reject block 22001.

Edit: U can remove all well-known peers and leave only ur own IP. Then ur node will talk to itself only and u will be able to send transactions that won't go outside.
gbeirn
Full Member
***
Offline Offline

Activity: 126
Merit: 100


View Profile
January 03, 2014, 06:17:59 PM
 #54

Ok, to be sure.  If I compile this code and run it, I should be able to download and process the blockchain up to block 30k?  It would be nice is there was a testnet for just for the source-- that way we could test-process transactions.

I have nodes up and running for a testnet but haven't been able to get anyone who was in the genesis block to give me a passphrase so we would have coins to use on it.

NXT VPS Server Donations can be sent here: 6044921191674841550
At the end of each month I will donate some of them back to the community.
This is separate from my main wallet so you can keep track of them. I will keep them in there and only use them for hosting.
Skerberus
Hero Member
*****
Offline Offline

Activity: 913
Merit: 664


View Profile
January 03, 2014, 06:24:26 PM
 #55

Here are some code-changes I would suggest:

The autoclosable-feature of Java 7 should be used, to prevent not closed streams in case of an exception:

Example:

Old Code:(805-815)

Code:
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();

}

better version (even closed in case of an exception during read):
Code:
try (FileInputStream fileInputStream = new FileInputStream(fileName);
ObjectInputStream objectInputStream = new ObjectInputStream(
fileInputStream);) {
blockCounter = objectInputStream.readInt();
blocks = (HashMap<Long, Block>) objectInputStream.readObject();
lastBlock = objectInputStream.readLong();
}

Please keep in mind that there are more place where the code should be changed this way (e.g. methods loadTransactions (3299), savetransactions (3437)...)

FrictionlessCoin
Legendary
*
Offline Offline

Activity: 868
Merit: 1000


Cryptotalk.org - Get paid for every post!


View Profile
January 03, 2014, 06:36:51 PM
 #56

Talk about amateur hour!!

So let's get this right,  the source code is released and we find it all in a single .java file.

One class with a lot of inner classes.   

Does the developer ever know what a java package is for?

Then he declares all the member variables of the Nxt class as static.  Does he know the difference between static and instance variable?

To place matters worse,  the Nxt class happens to be a servlet.  Does he not know that a new servlet is instantiated per thread??!

Well, to be honest, 21 BTC original invested may have been too generous!

 
                                . ██████████.
                              .████████████████.
                           .██████████████████████.
                        -█████████████████████████████
                     .██████████████████████████████████.
                  -█████████████████████████████████████████
               -███████████████████████████████████████████████
           .-█████████████████████████████████████████████████████.
        .████████████████████████████████████████████████████████████
       .██████████████████████████████████████████████████████████████.
       .██████████████████████████████████████████████████████████████.
       ..████████████████████████████████████████████████████████████..
       .   .██████████████████████████████████████████████████████.
       .      .████████████████████████████████████████████████.

       .       .██████████████████████████████████████████████
       .    ██████████████████████████████████████████████████████
       .█████████████████████████████████████████████████████████████.
        .███████████████████████████████████████████████████████████
           .█████████████████████████████████████████████████████
              .████████████████████████████████████████████████
                   ████████████████████████████████████████
                      ██████████████████████████████████
                          ██████████████████████████
                             ████████████████████
                               ████████████████
                                   █████████
.CryptoTalk.org.|.MAKE POSTS AND EARN BTC!.🏆
Skerberus
Hero Member
*****
Offline Offline

Activity: 913
Merit: 664


View Profile
January 03, 2014, 06:38:30 PM
 #57

Another code-change I would suggest:

Line 6795:

Old Code:

Code:
@Override
public void destroy() {

scheduledThreadPool.shutdown();
cachedThreadPool.shutdown();

try {

Block.saveBlocks("blocks.nxt");

} catch (Exception e) { }
try {

Transaction.saveTransactions("transactions.nxt");

} catch (Exception e) { }

try {

blockchainChannel.close();

} catch (Exception e) { }

logMessage("Nxt stopped.");

}


I would at least change it this way, because the Shutdown-Methods of scheduledThreadPool and cahedThreadPool can throw the following e.g. Exceptions ( java.lang.SecurityException,java.lang.RuntimePermission)
If one of these exceptions is thrown the blocks.nxt, transactions.nxt will not be saved.
Moreover it's not good style to leave catch-Blocks empty, so I would suggest to log those errors at least Smiley!
Code:
		@Override
public void destroy() {


try {
scheduledThreadPool.shutdown();


} catch (Exception e) {
logMessage(e.getMessage());
}

try {
cachedThreadPool.shutdown();
} catch (Exception e) {
logMessage(e.getMessage());

}




try {

Block.saveBlocks("blocks.nxt");

} catch (Exception e) {
logMessage(e.getMessage());
}
try {

Transaction.saveTransactions("transactions.nxt");

} catch (Exception e) {
logMessage(e.getMessage());
}

try {

blockchainChannel.close();

} catch (Exception e) {
logMessage(e.getMessage());
}

logMessage("Nxt stopped.");

}
Come-from-Beyond (OP)
Legendary
*
Offline Offline

Activity: 2142
Merit: 1010

Newbie


View Profile
January 03, 2014, 06:48:05 PM
 #58

Talk about amateur hour!!

So let's get this right,  the source code is released and we find it all in a single .java file.

One class with a lot of inner classes.   

Does the developer ever know what a java package is for?

Then he declares all the member variables of the Nxt class as static.  Does he know the difference between static and instance variable?

To place matters worse,  the Nxt class happens to be a servlet.  Does he not know that a new servlet is instantiated per thread??!

Well, to be honest, 21 BTC original invested may have been too generous!

All ur statements r irrelevant. They r just personal insults ("Does he know the difference between static and instance variable?") or incorrect guesses ("Does he not know that a new servlet is instantiated per thread?")
Trolls r not allowed here, so create another thread plz.
Come-from-Beyond (OP)
Legendary
*
Offline Offline

Activity: 2142
Merit: 1010

Newbie


View Profile
January 03, 2014, 06:49:30 PM
 #59

@Skerberus:

Thank u. If Jean-Luc has not fixed this yet, he will do it I believe.
FrictionlessCoin
Legendary
*
Offline Offline

Activity: 868
Merit: 1000


Cryptotalk.org - Get paid for every post!


View Profile
January 03, 2014, 06:51:07 PM
 #60

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

The flaws must be reported before the 3rd of April, after that date they can be revealed at any moment.

If u think that u found a flaw, post here its description. Mathematical proof is not necessary, common sense should be enough. If ur guess is correct u may* get the reward, if u find a non-injected flaw then u'll be asked for more formal proof (u may get a reward too).

NB: Some guys mentioned that they would just decompile 0.4.7e binaries and compare the source codes to find the flaws. As a countermeasure against such the trick u still must explain why there is a flaw.

-------------
* - BCNext reserves the right to refuse to pay a reward without any explanation. This is an anti-troll countermeasure.


Man,  the B.S. is unbelievable.

 
                                . ██████████.
                              .████████████████.
                           .██████████████████████.
                        -█████████████████████████████
                     .██████████████████████████████████.
                  -█████████████████████████████████████████
               -███████████████████████████████████████████████
           .-█████████████████████████████████████████████████████.
        .████████████████████████████████████████████████████████████
       .██████████████████████████████████████████████████████████████.
       .██████████████████████████████████████████████████████████████.
       ..████████████████████████████████████████████████████████████..
       .   .██████████████████████████████████████████████████████.
       .      .████████████████████████████████████████████████.

       .       .██████████████████████████████████████████████
       .    ██████████████████████████████████████████████████████
       .█████████████████████████████████████████████████████████████.
        .███████████████████████████████████████████████████████████
           .█████████████████████████████████████████████████████
              .████████████████████████████████████████████████
                   ████████████████████████████████████████
                      ██████████████████████████████████
                          ██████████████████████████
                             ████████████████████
                               ████████████████
                                   █████████
.CryptoTalk.org.|.MAKE POSTS AND EARN BTC!.🏆
Pages: « 1 2 [3] 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 ... 65 »
  Print  
 
Jump to:  

Powered by MySQL Powered by PHP Powered by SMF 1.1.19 | SMF © 2006-2009, Simple Machines Valid XHTML 1.0! Valid CSS!