Bitcoin Forum
August 22, 2017, 05:47:38 AM *
News: Latest stable version of Bitcoin Core: 0.14.2  [Torrent].
 
   Home   Help Search Donate Login Register  
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 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 »
  Print  
Author Topic: Nxt source code flaw reports  (Read 108274 times)
Come-from-Beyond
Legendary
*
Offline Offline

Activity: 1680

Newbie


View Profile
January 06, 2014, 10:33:55 AM
 #521

Lol, are you really sure that the published code on bitbucket contains 3 flaws?  Grin
I have done some local blackbox-testing with 2 machines and everything works fine.

Furthermore I have checked the classes User, Peer, Transaction, Block, Account and the method init, doGet and everything looks very clean there (beside the slightly odd use of oop that was discussed here enough). Some more classes to check..let's go.

It does contain these flaws. Logical flaws, not programming bugs.
1503380858
Hero Member
*
Offline Offline

Posts: 1503380858

View Profile Personal Message (Offline)

Ignore
1503380858
Reply with quote  #2

1503380858
Report to moderator
1503380858
Hero Member
*
Offline Offline

Posts: 1503380858

View Profile Personal Message (Offline)

Ignore
1503380858
Reply with quote  #2

1503380858
Report to moderator
Advertised sites are not endorsed by the Bitcoin Forum. They may be unsafe, untrustworthy, or illegal in your jurisdiction. Advertise here.
1503380858
Hero Member
*
Offline Offline

Posts: 1503380858

View Profile Personal Message (Offline)

Ignore
1503380858
Reply with quote  #2

1503380858
Report to moderator
ZeroTheGreat
Hero Member
*****
Offline Offline

Activity: 518


View Profile
January 06, 2014, 10:36:20 AM
 #522

Lol, are you really sure that the published code on bitbucket contains 3 flaws?  Grin
I have done some local blackbox-testing with 2 machines and everything works fine.

Furthermore I have checked the classes User, Peer, Transaction, Block, Account and the method init, doGet and everything looks very clean there (beside the slightly odd use of oop that was discussed here enough). Some more classes to check..let's go.
Logical flaws.
smaragda
Legendary
*
Offline Offline

Activity: 1386



View Profile
January 06, 2014, 10:57:56 AM
 #523


Can someone point to the part of the code where it is SECURED that account IDs, publicKeys, etc., which are being analyzed (whether it be for blocks, transactions, payments, etc.), have been created WITHIN the NXT network?

It is clear that even to connect to the NXT network (not to mention synchronize, analyze, etc.) one needs to go back to the Genesis Block in one way or the other, but, for some reason, I can't find an answer to my question above.


Edit:  PLEASE!   Wink Cheesy Wink

Simcoin (SIM) & CryptoPlay (CPS) DEV NxtChg IS A FUCKIN' LOW-LIFE PIECE OF SHIT EMBEZZLING PSYCHOPATH   Angry
https://bitcointalk.org/index.php?topic=929688.msg10205597#msg10205597                    "A world with the money can not be perfect." - BCNext
BloodyRookie
Hero Member
*****
Offline Offline

Activity: 673


View Profile
January 06, 2014, 11:58:53 AM
 #524

Lol, are you really sure that the published code on bitbucket contains 3 flaws?  Grin
I have done some local blackbox-testing with 2 machines and everything works fine.

Furthermore I have checked the classes User, Peer, Transaction, Block, Account and the method init, doGet and everything looks very clean there (beside the slightly odd use of oop that was discussed here enough). Some more classes to check..let's go.
Logical flaws.

Yes, logical flaws. But if I understand CfB correct, he states that the original 0.4.7e (the version which was distributed to the nxt users) and subsequent versions do NOT contain those logical flaws. Can you confirm that, CfB?

Nothing Else Matters
NEM: NALICE-LGU3IV-Y4DPJK-HYLSSV-YFFWYS-5QPLYE-ZDJJ
NXT: 11095639652683007953
bitcoinpaul
Hero Member
*****
Offline Offline

Activity: 798



View Profile
January 06, 2014, 12:00:10 PM
 #525

yes, of course.

edit: Grin
Come-from-Beyond
Legendary
*
Offline Offline

Activity: 1680

Newbie


View Profile
January 06, 2014, 12:01:19 PM
 #526

Yes, logical flaws. But if I understand CfB correct, he states that the original 0.4.7e (the version which was distributed to the nxt users) and subsequent versions do NOT contain those logical flaws. Can you confirm that, CfB?

No, I can't.
BloodyRookie
Hero Member
*****
Offline Offline

Activity: 673


View Profile
January 06, 2014, 12:10:14 PM
 #527

hmpf....so I misunderstood the phrase "3 flaws were injected into 0.4.7e".

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

Activity: 1680

Newbie


View Profile
January 06, 2014, 12:16:49 PM
 #528

hmpf....so I misunderstood the phrase "3 flaws were injected into 0.4.7e".

0.4.7e source code was modified before publishing.
smaragda
Legendary
*
Offline Offline

Activity: 1386



View Profile
January 06, 2014, 12:25:38 PM
 #529


Are there any security concerns for BigInteger two64 = new BigInteger("18446744073709551616"); to be out in the open?

How/Why is that number chosen?  ...I wonder if this can be related to my previous question too...   Huh

Simcoin (SIM) & CryptoPlay (CPS) DEV NxtChg IS A FUCKIN' LOW-LIFE PIECE OF SHIT EMBEZZLING PSYCHOPATH   Angry
https://bitcointalk.org/index.php?topic=929688.msg10205597#msg10205597                    "A world with the money can not be perfect." - BCNext
Come-from-Beyond
Legendary
*
Offline Offline

Activity: 1680

Newbie


View Profile
January 06, 2014, 12:27:00 PM
 #530


Are there any security concerns for BigInteger two64 = new BigInteger("18446744073709551616"); to be out in the open?

How/Why is that number chosen?  ...I wonder if this can be related to my previous question too...   Huh

No. It's just 264.
ricot
Jr. Member
*
Offline Offline

Activity: 56


View Profile
January 06, 2014, 12:47:32 PM
 #531

So since my local 0.5.0 clients started crashing (i.e. showing negative recent blocks numbers, etc.) I started analyzing a little...
I started by getting the blocks that it currently knows from the api...
The block ids from the top of the list were:
16120160395225814370
17250788183583576225
11124825242748196662
14790117364446866715
8810693157391937532

So according to the block explorer, my top 4 blocks are from a forked chain. Nothing tragic, happens...
Let's look at the cumulative difficulty:
my client says 877033680606371.
Let's check some other node in the network, take node10.nxtbase.com, and we get: 877619990555689

So why doesn't my client catch up?
Let's have a look with wireshark, what's going on...
The first thing that we see is... ehm... 192.161.48.32, aka node00.nxtbase.org.
This node is spamming me with all kinds of slightly weird stuff, e.g. duplicate acks.
Ok, whatever, let's look at our stuff...
My client seems to only do Post requests to one single address... 192.161.48.32 WTF?!?
The address answers fast with a nice cumulative difficulty of 866067307974274.
Wait a sec... that's quite low? What's going on there? Well, later...
First: Why the hell am I only asking that weird address???
Let's see whom I'm connected to. getPeers from my account and for each of them getPeer and look for connected:1, because only from them I will query new blocks.
There are 10 of them, one is node00.nxtbase.org, the other 9 look a bit more legit... but wait!
All of those 9 have a negative weight... doesn't that? Yes, yes, it does... If a node's weight is negative, it isn't considered for blockchain scanning. Ok, but then, why isn't it connecting to other nodes?
Well, it only connects to other nodes if it isn't connected to 10 nodes already... And why does it keep the connection to nodes with a negative weight? I don't know... Really, why don't you cancel connections to nodes that you'll never use???
Anyways... How did the weight become negative?
Let's start by looking at peer.weight... This is set in analyze hallmark, and only there... and before it is set, it's checked whether it is smaller than 0... WTF?!? Ah! getWeight() is not a getter for weight... well, obviously, why would I expect any senseful naming from that code! Thanks for taking my time looking into the wrong direction, again. (shift+alt+r getBalanceAdjustedWeight) [for those, that don't know eclipse: i just renamed all occurrences of getWeight with getBalanceAdjustedWeight, what it really is, so I won't be confused by that in the future... Why CfB and others didn't do that? IDK.]
But on that topic, I also saw an updateWeight... hmm, is that going to update the weight or the adjustedWeight? Well, neither, it doesn't take a parameter and it sends a processNewData request to all Users... Guys, seriously?!? MVC - Model, View, Controller, not Mix Various Crap :p
Back to our attacker, or the weight, or sth...
How can it be negative?
Well, it calculates the following: adjustedWeight * (account.getBalanceCent() / 100) / 1000000000 [I added Cent to the functions that return amount in 0.01 NXT, because some did, some didn't]
adjustedWeight is calculated whenever a new hallmark is analyzed and it is basically just the fraction of this peer's weight within all peer's weight, nothing that could be negative.
But the balance might be... at least I know one account off the top of my head that has a negative balance, the genesis account.
So let's take a node with a negative balance and look at the account.
Randomness has chosen: node81.nxtbase.com with a current weight of -25046. So which account does it have?
Well, the account ID are the first 32 bytes of the hallmark, well , actually the last 32, but let's not go into that topic...
Anyway, the accountID is 11243542237777034551, so it's not the genesis account and it should have a balance of around 315k according to the block explorer. So what does my client say?
Well, it's not that easy. Unfortunately we can't query an account's balance with any API call... bummer.
So let's look at the transactions instead... All of them have a lot of confirmations, so just accumulate them, subtract the fees for sent transactions... aaaand... 315402... looks legit, definitely not negative.
So what happent to Account.balance?
Let's look at where that is set. We've got a total of 6 occurrences...
When we analyze a block:
- the generator account is credited with the fee. (Let's hope noone gets credited more than once)
- for every transaction, the sender account gets the transaction amount and fee deducted (the sender account is expected to already exist, let's hope that's always the case and doesn't crash with a NullPointer because then the recipients wouldn't get their balance updated)
- for every transaction, the recipient is credited with the transaction amount
When we popLastBlock:
Same procedure, just all the + are now -. Say something about duplicate code, but hey...

Ok, so far so good, let's go deeper Smiley
Let's change the order of things and first look at where popLastblock is called... because that gets only called once. Wink
It's called when we're getting a block chain, in case the last common block of us and the peer is not either of our last blocks, i.e. there has been a fork. Should happen regularly, right?
So what do we do in that case?
We write blocks and transactions to a backup file, then we pop our blocks until we find the common ancestor, then we push the peers blocks, and then we check if the cumulative difficulty really has increased, as promised earlier by the peer.
So what happens, if the peer lied about the cumulative difficulty and just sent us a few (say 2) blocks, branched from very far (say 700) blocks back?
We'll pop 700 blocks, push 2 blocks, realize that we got bulls^&t, blacklist the peer and load our backups. And then everything is fine, isn't it? ISN'T IT?
No, it's not.
While popping the 700 blocks, we adjusted the balance of all accounts to the state 700 blocks ago. Then when pushing the 2 blocks, we took whatever transactions the "intruder" wanted us to know and included into his blocks, and adjust our account's balances accordingly.
And when we realize, everything was crap? Do we reset the accounts to what they were before? No, we don't. We keep the crap balances and hope for the best.

So if you want to take down a client, that is *any client*, just analyze the hall marks in the network, wait for some nice transactions, and generate a few blocks where you may or may not include the transactions to set the balance to a negative value which then causes the attacked client to not talk to that part of the network anymore and the *validation of new blocks to also be corrupted*, which enables you to send some nicely crafted blocks that only the attacked clients will accept and strengthen your position as the only peer it wants to communicate with.

So is that the injected flaw to take down a forked currency within a day? If yes, why does it work in 0.5.0? ^^

(Btw: Only posting this publicly because it is already widely used)


[edit]
Totally forgot: Kudos to whoever attacked using that way, nice find. Smiley

[edit2]
@immortAlex: That's what's causing the negative numbers...
smaragda
Legendary
*
Offline Offline

Activity: 1386



View Profile
January 06, 2014, 12:47:51 PM
 #532

No. It's just 264.

Was my previous question EVEN MORE trivial for you to not even bother?   Cheesy

Simcoin (SIM) & CryptoPlay (CPS) DEV NxtChg IS A FUCKIN' LOW-LIFE PIECE OF SHIT EMBEZZLING PSYCHOPATH   Angry
https://bitcointalk.org/index.php?topic=929688.msg10205597#msg10205597                    "A world with the money can not be perfect." - BCNext
ImmortAlex
Hero Member
*****
Offline Offline

Activity: 756


View Profile
January 06, 2014, 12:48:08 PM
 #533

Today I got famous "Recent blocks [-nnnnn]" in client interface. Two times in row.
So I start lurking sources where pushBlock() and popBlock() called.

Lets assume doPost() with "processBlock" executes simutaneously with thread that load blocks from some peer (line 4463 and below).
That two code branches is not syncronized on blocks completely.
Loading thread can do a lot of changes in local blockchain, not only appending new blocks, but reorganizing of chain tail in case of choosing different fork.
doPost() at the same time can process block from request almost completely. It calls pushBlock() that:
- create block
- check it validity (!)
- check block.previousBlock == lastBlock (!!)
- sync on blocks, waiting for loading thread complete (!!!)
- reset lastBlock to it's block, completely ignore what loading thread done (!!!!)

Seems like next call of loading thread can't fix that broken blockchain and f*ck up in loop at line 4642
Code:
while (lastBlock != commonBlockId && Block.popLastBlock()) { }
which cause "Recent blocks [-nnnnn]" I mention in beginning of this post.

Am I miss something?

Edit: ricot lurking for the same in the beginning Cheesy

NXT: NXT-VDTM-HDPP-XTAX-CEQF3
BTC: 1A6t7HRPDhd31ze4iahuQoEVdeAqx8Jhyv
gsan1
Jr. Member
*
Offline Offline

Activity: 50


View Profile
January 06, 2014, 12:49:44 PM
 #534

hmpf....so I misunderstood the phrase "3 flaws were injected into 0.4.7e".

0.4.7e source code was modified before publishing.

Huh The binary of 0.4.7e that was published weeks ago and used in production does not contain the flaws we are searching here, right? Otherwise that would mean, that we used software that has critical flaws in it. The binary 0.5.0 should have these flaws fixed to, right? Otherwise someone could exploit them, when they are known in this topic.

The source code of 0.4.7e that was published on 3. January is the version of binary 0.4.7e minus alias, coloured coins plus three security flaws. Right? So why can't you confirm that the flaws we are searching are fixed in 0.4.7e or 0.5.0?
ferment
Full Member
***
Offline Offline

Activity: 168


View Profile
January 06, 2014, 12:53:24 PM
 #535

hmpf....so I misunderstood the phrase "3 flaws were injected into 0.4.7e".

0.4.7e source code was modified before publishing.

The source code of 0.4.7e that was published on 3. January is the version of binary 0.4.7e minus alias, coloured coins plus three security flaws. Right? So why can't you confirm that the flaws we are searching are fixed in 0.4.7e or 0.5.0?

To get the published code:

1. Remove advanced stuff (aliases, etc) from 0.4.7e.
2. Inject 3 logic flaws.
3. Post on bitbucket.

The code for 0.5.0 never saw the flaws.

ImmortAlex
Hero Member
*****
Offline Offline

Activity: 756


View Profile
January 06, 2014, 12:54:41 PM
 #536

So why can't you confirm that the flaws we are searching are fixed in 0.4.7e or 0.5.0?
There was no flaws we are looking for. They were introduced in source code with purpose.

NXT: NXT-VDTM-HDPP-XTAX-CEQF3
BTC: 1A6t7HRPDhd31ze4iahuQoEVdeAqx8Jhyv
gsan1
Jr. Member
*
Offline Offline

Activity: 50


View Profile
January 06, 2014, 12:59:18 PM
 #537

To get the published code:

1. Remove advanced stuff (aliases, etc) from 0.4.7e.
2. Inject 3 logic flaws.
3. Post on bitbucket.

The code for 0.5.0 never saw the flaws.

But this is a condradiction to CfBs statement?

Yes, logical flaws. But if I understand CfB correct, he states that the original 0.4.7e (the version which was distributed to the nxt users) and subsequent versions do NOT contain those logical flaws. Can you confirm that, CfB?

No, I can't.
Come-from-Beyond
Legendary
*
Offline Offline

Activity: 1680

Newbie


View Profile
January 06, 2014, 12:59:43 PM
 #538

So since my local 0.5.0 clients started crashing (i.e. showing negative recent blocks numbers, etc.) I started analyzing a little...
I started by getting the blocks that it currently knows from the api...
The block ids from the top of the list were:
16120160395225814370
17250788183583576225
11124825242748196662
14790117364446866715
8810693157391937532

So according to the block explorer, my top 4 blocks are from a forked chain. Nothing tragic, happens...
Let's look at the cumulative difficulty:
my client says 877033680606371.
Let's check some other node in the network, take node10.nxtbase.com, and we get: 877619990555689

So why doesn't my client catch up?
Let's have a look with wireshark, what's going on...
The first thing that we see is... ehm... 192.161.48.32, aka node00.nxtbase.org.
This node is spamming me with all kinds of slightly weird stuff, e.g. duplicate acks.
Ok, whatever, let's look at our stuff...
My client seems to only do Post requests to one single address... 192.161.48.32 WTF?!?
The address answers fast with a nice cumulative difficulty of 866067307974274.
Wait a sec... that's quite low? What's going on there? Well, later...
First: Why the hell am I only asking that weird address???
Let's see whom I'm connected to. getPeers from my account and for each of them getPeer and look for connected:1, because only from them I will query new blocks.
There are 10 of them, one is node00.nxtbase.org, the other 9 look a bit more legit... but wait!
All of those 9 have a negative weight... doesn't that? Yes, yes, it does... If a node's weight is negative, it isn't considered for blockchain scanning. Ok, but then, why isn't it connecting to other nodes?
Well, it only connects to other nodes if it isn't connected to 10 nodes already... And why does it keep the connection to nodes with a negative weight? I don't know... Really, why don't you cancel connections to nodes that you'll never use???
Anyways... How did the weight become negative?
Let's start by looking at peer.weight... This is set in analyze hallmark, and only there... and before it is set, it's checked whether it is smaller than 0... WTF?!? Ah! getWeight() is not a getter for weight... well, obviously, why would I expect any senseful naming from that code! Thanks for taking my time looking into the wrong direction, again. (shift+alt+r getBalanceAdjustedWeight) [for those, that don't know eclipse: i just renamed all occurrences of getWeight with getBalanceAdjustedWeight, what it really is, so I won't be confused by that in the future... Why CfB and others didn't do that? IDK.]
But on that topic, I also saw an updateWeight... hmm, is that going to update the weight or the adjustedWeight? Well, neither, it doesn't take a parameter and it sends a processNewData request to all Users... Guys, seriously?!? MVC - Model, View, Controller, not Mix Various Crap :p
Back to our attacker, or the weight, or sth...
How can it be negative?
Well, it calculates the following: adjustedWeight * (account.getBalanceCent() / 100) / 1000000000 [I added Cent to the functions that return amount in 0.01 NXT, because some did, some didn't]
adjustedWeight is calculated whenever a new hallmark is analyzed and it is basically just the fraction of this peer's weight within all peer's weight, nothing that could be negative.
But the balance might be... at least I know one account off the top of my head that has a negative balance, the genesis account.
So let's take a node with a negative balance and look at the account.
Randomness has chosen: node81.nxtbase.com with a current weight of -25046. So which account does it have?
Well, the account ID are the first 32 bytes of the hallmark, well , actually the last 32, but let's not go into that topic...
Anyway, the accountID is 11243542237777034551, so it's not the genesis account and it should have a balance of around 315k according to the block explorer. So what does my client say?
Well, it's not that easy. Unfortunately we can't query an account's balance with any API call... bummer.
So let's look at the transactions instead... All of them have a lot of confirmations, so just accumulate them, subtract the fees for sent transactions... aaaand... 315402... looks legit, definitely not negative.
So what happent to Account.balance?
Let's look at where that is set. We've got a total of 6 occurrences...
When we analyze a block:
- the generator account is credited with the fee. (Let's hope noone gets credited more than once)
- for every transaction, the sender account gets the transaction amount and fee deducted (the sender account is expected to already exist, let's hope that's always the case and doesn't crash with a NullPointer because then the recipients wouldn't get their balance updated)
- for every transaction, the recipient is credited with the transaction amount
When we popLastBlock:
Same procedure, just all the + are now -. Say something about duplicate code, but hey...

Ok, so far so good, let's go deeper Smiley
Let's change the order of things and first look at where popLastblock is called... because that gets only called once. Wink
It's called when we're getting a block chain, in case the last common block of us and the peer is not either of our last blocks, i.e. there has been a fork. Should happen regularly, right?
So what do we do in that case?
We write blocks and transactions to a backup file, then we pop our blocks until we find the common ancestor, then we push the peers blocks, and then we check if the cumulative difficulty really has increased, as promised earlier by the peer.
So what happens, if the peer lied about the cumulative difficulty and just sent us a few (say 2) blocks, branched from very far (say 700) blocks back?
We'll pop 700 blocks, push 2 blocks, realize that we got bulls^&t, blacklist the peer and load our backups. And then everything is fine, isn't it? ISN'T IT?
No, it's not.
While popping the 700 blocks, we adjusted the balance of all accounts to the state 700 blocks ago. Then when pushing the 2 blocks, we took whatever transactions the "intruder" wanted us to know and included into his blocks, and adjust our account's balances accordingly.
And when we realize, everything was crap? Do we reset the accounts to what they were before? No, we don't. We keep the crap balances and hope for the best.

So if you want to take down a client, that is *any client*, just analyze the hall marks in the network, wait for some nice transactions, and generate a few blocks where you may or may not include the transactions to set the balance to a negative value which then causes the attacked client to not talk to that part of the network anymore and the *validation of new blocks to also be corrupted*, which enables you to send some nicely crafted blocks that only the attacked clients will accept and strengthen your position as the only peer it wants to communicate with.

So is that the injected flaw to take down a forked currency within a day? If yes, why does it work in 0.5.0? ^^

(Btw: Only posting this publicly because it is already widely used)


[edit]
Totally forgot: Kudos to whoever attacked using that way, nice find. Smiley

This is a serious bug and is worth a reward. Thank u.

PS: This is not an injected flaw.
Come-from-Beyond
Legendary
*
Offline Offline

Activity: 1680

Newbie


View Profile
January 06, 2014, 01:02:53 PM
 #539

No. It's just 264.

Was my previous question EVEN MORE trivial for you to not even bother?   Cheesy

Maybe...
ImmortAlex
Hero Member
*****
Offline Offline

Activity: 756


View Profile
January 06, 2014, 01:03:29 PM
 #540

PS: This is not an injected flaw.
It's time to post this:

NXT: NXT-VDTM-HDPP-XTAX-CEQF3
BTC: 1A6t7HRPDhd31ze4iahuQoEVdeAqx8Jhyv
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 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 »
  Print  
 
Jump to:  

Sponsored by , a Bitcoin-accepting VPN.
Powered by MySQL Powered by PHP Powered by SMF 1.1.19 | SMF © 2006-2009, Simple Machines Valid XHTML 1.0! Valid CSS!