nanocurrency / nano-node

Nano is digital currency. Its ticker is: XNO and its currency symbol is: Ӿ
https://nano.org
BSD 3-Clause "New" or "Revised" License
3.48k stars 787 forks source link

Would Raiblocks applications (malicious or not) be vulnerable to private key leaking? #385

Closed eddieoz closed 6 years ago

eddieoz commented 6 years ago

I was looking about the sign algorithm Ed25519 and found this issue: https://github.com/jedisct1/libsodium/issues/170

Lets imagine the flow below:

1) Origin wallet sign a transaction and broadcast it 2) Some bit(s) could flip during the signing 3) Destination wallet receives the transaction and invalidates it because of wrong signature 4) Origin wallet creates the same transaction again and re-broadcasts it 5) Destination wallet receives the transaction ok. And as a prize, it could extract the origin wallet's private key

Having some bits changed during the signing happens, a lot. And happens more on mobile devices, mostly on iOS for example.

It could expose the private key, not just to the destination, but for all network.

ianhattendorf commented 6 years ago

Interesting. Looks like the defence against this would be to verify the signature before sending a transaction, and error out/re-sign if the signature is invalid?

pocesar commented 6 years ago

well, the issuer should never be trusted. can't expected always-in-shape data flying across the network...

ianhattendorf commented 6 years ago

Right. After the origin generates a bit-flipped transaction, it would try to validate its own transaction. If the validation fails, it knows that a bit got flipped. It should then toss that signature and re-sign, validate again, and then broadcast the transaction. This way, the bit-flipped transaction never leaves the origin wallet. This is assuming that the node is the "bad actor", and origin is the innocent user.

pocesar commented 6 years ago

from an innocent user perspective, yes, the wallet itself should warn. but attacks like these will be coming from crafted packets outside of the network, from third party code (easy to use tools like metasploit)

cryptocode commented 6 years ago

Having some bits changed during the signing happens, a lot.

Citation needed.

eddieoz commented 6 years ago

Yep, that was discussed on the libsodium thread. I think the time for validate is high, but security would came first. Validation after signing really could help.

Imagine an exchange, doing a lot of transactions, probably could have more chances on happening that.

cryptocode commented 6 years ago

I would assume exchanges run on sane hardware with ECC.

eddieoz commented 6 years ago

Citation needed.

We developed a lib for managing btc private-keys and signing/validating messages (as proof-of-work) on react-native, running on android and iOS. We had a lot of problems on iOS because the way Buffer lib manages the memory, causing random invalid signatures.

ianhattendorf commented 6 years ago

Having some bits changed during the signing happens, a lot.

Citation needed.

https://media.blackhat.com/bh-us-11/Dinaburg/BH_US_11_Dinaburg_Bitsquatting_WP.pdf

Not a lot per person, but aggregated it could certainly be considered "a lot" if the result is someone potentially losing their money.

eddieoz commented 6 years ago

Citation needed.

I like this too:

Scientists believe the cosmic radiation might get bounced off nearby star systems. Studies by IBM in the 1990s suggest that computers typically experience about one cosmic-ray-induced error per 256 megabytes of RAM per month, however, scientists have speculated that outages could increase as chip sizes get smaller. Where bit errors such as those caused by cosmic radiation can sometimes impact two units of data, putting a buffer between cells can prevent the error from spreading.

https://www.inverse.com/article/21293-cisco-outage-cosmic-radiation-wtf and https://www.reddit.com/r/networking/comments/53qw9n/cisco_100g_line_cards_silently_dropping_traffic/

It just happens.

pocesar commented 6 years ago

i'm kinda baffled that the node take it as-is? is it to not use processing power? (how low could it get)

@eddieoz for the bounty, you need to reach them on discord btw

eddieoz commented 6 years ago

Is there a bounty for reporting? interesting. I'll go there. Thx @pocesar

pocesar commented 6 years ago

it's mentioned here https://medium.com/@clemahieu/announcing-the-raiblocks-bug-bounty-program-b7185711bd52

ianhattendorf commented 6 years ago

If the details of the bug leak ahead of the retrospective being published, whether accidentally or maliciously, the contract between RaiBlocks and the reporter is null-and-void and the bug bounty will not be rewarded.

Although I wonder what that means for duplicate reports (someone reports confidentially, then someone else discovers and posts it online).

eddieoz commented 6 years ago

hm, I applied to the form they published on discord. Lets see what they think about it.

cryptocode commented 6 years ago

I think the rule is not to publish it first. It's important to discover these things at any rate.

eddieoz commented 6 years ago

@cryptocode there is no problem if I don't receive any bounty or incentive for looking issues, even a very critical like that.

You are worried about very low priorities on this thread. 'lot', 'bounties', etc. Don't worry about me. I'm sure you can contribute more deeply. Stay nice.

cryptocode commented 6 years ago

@eddieoz I was merely asking citations ;) That was provided, which is much appreciated.

eddieoz commented 6 years ago

I'm really sorry for my misunderstanding! Let's carry on trying to make cryptos safer and better :) Regards

SergiySW commented 6 years ago

If origin wallet will create invalid signature it won't be broadcasted to the network because cannot pass signature check in ledger processor https://github.com/clemahieu/raiblocks/blob/7baff455529d05978b81f850acc66d4847ea06a1/rai/secure.cpp#L2366 https://github.com/clemahieu/raiblocks/blob/7baff455529d05978b81f850acc66d4847ea06a1/rai/secure.cpp#L2399 ...

clemahieu commented 6 years ago

It's an interesting vulnerability.

Looking at the implementation in wallet.cpp, the relevant block creation functions are in send_action, receive_action, and change_action. Both of them process the block through process_receive_many which first checks the block for validity, including signature, before it would be published out, just as if it had been received off the network.

From what I can see there isn't a path that broadcasts a transaction before it has a local signature verification check.

brunoerg commented 6 years ago

Colin, one question.

If I build the node or the wallet but before it I remove the verification lines from the source code. Would the node/wallet countinue working?

Because if the answer is "yes". I can do a nice wallet (nice design, ux/ui, and other), and how the current wallet doesn't have a nice (UI/UX), the people could start using my wallet and then, after a time, I could rob them.

What do you think about this situation?

ianhattendorf commented 6 years ago

The chance of this specific bug happening would still be extremely small per person. If people trust you and are using your wallet, there are much easier ways than relying on a bit to flip when signing a transaction.

That's why it's so important to only use wallets you trust. Not just for RaiBlocks, but any cryptocurrency. And not just wallets, but any software. Because if anything is compromised, you can lose your private keys for all of your crypto, and anything else private on your computer.

androm3da commented 6 years ago

The time described is the period between Key Generation and Message Signing, which spans much more than a single message or even a single continuous uptime session of a node.

Any time the key is used for signing, this exposure exists. Unfortunately performing signature verification verifies against the key in-memory which may not be the same as the key at Key Generation, if the key's persistent store has been corrupted.

IIUC the key persisted is the encrypted representation of the key which is verified against a check value. That should mitigate this risk.

pocesar commented 6 years ago

that raises the question, it IS vulnerable to Meltdown and Spectre then

PlasmaPower commented 6 years ago

Meltdown and spectre are fixed by compilers and operating systems, not this code. They're different than this issue though, since both meltdown and spectre can only read memory, not change it.

Encrypting the key in memory wouldn't help, because the key could just be read from memory too. However, it might be worth looking into Linux keyrings: http://man7.org/linux/man-pages/man7/keyrings.7.html

augustresende commented 6 years ago

They are using this issue in the Brazilian community to spread FUD saying that there is a "serious vulnerability" in the Nano code that exposes the private key without explaining anything.

augustresende commented 6 years ago

This is an issue that does not exist in a practical way and MAY only exist in poorly programmed light wallets.

joeldo commented 6 years ago

This Ed25519 flaw is very similar to a ECDSA vulnerability, which in 2013 caused the theft of about 59 Bitcoins. Android wallets were using poor RNG to seed their signature scheme and ended up exposing their private keys. Back then, this wasn't even considered a vulnerability of the Bitcoin protocol or infrastructure.

For both protocols, in the event of a key disclosure because of an exploit, a faulty client/node can only compromise itself and not the entire system.