Bitcoin Forum
November 15, 2024, 01:38:31 AM *
News: Latest Bitcoin Core release: 28.0 [Torrent]
 
   Home   Help Search Login Register More  
Pages: [1]
  Print  
Author Topic: Buggy CVE-2013-4627 patch, open new vectors of attack  (Read 869 times)
Sergio_Demian_Lerner (OP)
Hero Member
*****
expert
Offline Offline

Activity: 555
Merit: 654


View Profile WWW
July 19, 2013, 06:40:48 PM
 #1

After talking with the core dev about this issue, their opinion is that this is not a security threat, nor a vulnerability. Accordingly, it's best for the community to be informed about the problem so each user or organization evaluates if upgrading to the 0.8.3 version (instead of using the previous version, vulnerable to CVE-2013-4627) is desirable or not, depending on risks associated with their own Bitcoin infrastructure.

The original disclosure can be found at: https://bitslog.wordpress.com/2013/07/18/buggy-cve-2013-4627-patch-open-new-vectors-of-attack/

Here I'm copying the contents so bitcointalk users do not need to click on the link:

Buggy CVE-2013-4627 patch, open new vectors of attack

Secure coding is hard. But in Bitcoin, secure coding also means understanding every little detail of the undocumented (or code-documented) rules that Satoshi the great has brought to us mortals. CVE-2013-4627 patches a DoS vulnerability discovered by Peter Todd. The vulnerability is easy to spot once you read the code after the patch was applied. Bitcoin network messages can be of arbitrary sizes, and this is generally not a problem, with the sole exception of transaction messages. Almost all objects that are exchanged between Satoshi clients are deserialized to be stored in memory and serialized again when they need to be forwarded. . Transactions are also first deserialized in order to be parsed, but they are stored “as is” in a temporary cache called MapRelay, and forwarded without being serialized again. I really don’t know why this is done. But the point is that the storage of messages as they are received leads to the possibility that an attacker creates messages much bigger than the actual transaction object contained, which, when stored in MapRelay memory, can be used to exhaust the memory of a node. This is the code that patches the vulnerability:


Code:
unsigned int nSize = ::GetSerializeSize(tx, SER_NETWORK, PROTOCOL_VERSION);
unsigned int oldSize = vMsg.size();

if (nSize < oldSize) {
  vMsg.resize(nSize); ...
}

This patch that supposedly solves the CVE-2013-4627 bug opens the door at least two other attacks. Transactions on the wire may have an encoding which is different from the encoding you get when you serialize and deserialize it.
Still after many efforts to standardize transactions, preventing ambiguity and storing signatures in a canonical form, transactions are malleable. So when you truncate vMsg, you don’t know what are you truncating.

Double-spend Attack

The attack works against sites that accept 0 confirmations like SatoshiDice. It could allow 100% effective double-spends. The idea is that you create a message vMsg with a transaction Tx such that:

Code:
vMsg.size() == n
nSize = ::GetSerializeSize(tx, SER_NETWORK, PROTOCOL_VERSION) == n-1

But in the transaction stored in vMsg, the field vin.size is stored in a non-standard format that takes 4 bytes more.

Suppose that vin.size = 1. Then it is stored as: 0xfe 0×01 0×00 0×00 0×00, instead of being stored as “0×01″

When vMsg is truncated by vMsg.resize(nSize), it will become an invalid transaction (since 3 good bytes at the end will be chopped off).
Now the node that receives vMsg will process the Tx normally, but will relay the transaction vMsg (which is invalid). This is because the invalid tx in the msg will be stored in MapRelay for 15 minutes. Peers will ignore this invalid message. If the node processes Tx and responds with a transaction Tx’ (using Tx outputs, such as SatoshiDice) then Tx’ will end up in peer’s orphan cache, because Tx is not present since it wasn’t accepted.

So the attack works with 100% success rate for sites that create transactions as response to unconfirmed transactions, such as SatoshiDice, and are running version 0.8.3 of the Satoshi client.

Suppose that a site called InstantBitBets uses version 0.8.3 of the Satoshi client. The attack against InstantBitBets looks like this:

1. Locate the node of InstantBitBets. Connect directly to this node.
2. Create a bet in a Tx. Create a non-standard message vMsg with 1 extra byte and a non-standard  vin.size field.
3. Send it directly to InstantBitBets node.
4. If you receive a Tx’ saying that you won, broadcasts Tx to the rest of the network (normally).
5. If you’ve lost, move the funds from Tx to Tx2, and try again from Tx2 (this step might not even be necessary).

Also the attack could help to execute other kind of unknown attacks, since everybody expects a node to broadcasts successfully a Tx that it accepts.

Remote Anonymous Denial of Service Attack

If the right mix of Bitcoinj, old Satoshi nodes and new 0.8.3 nodes are present in the network, then a new attack vector emerges that is able to disconnect 0.8.3 nodes from Bitcoinj nodes remotely.
Consider, for example,  the following mix: old nodes=50%, 0.8.3 nodes=40%, Bitcoinj nodes=10%.

The attacker builds a transaction with an expanded var_int field (e.g.: input count). The expansion consist of encoding the count in more bytes than it is required. This transaction is forwarded to a bunch of 0.8.1 and prior clients. Alternatively, transactions passing through the attackers node can be modified on-the-fly, expanding the input count fields. Each of these clients will resend the tx without modification, and the tx will spread. When the transaction arrives to an old Satoshi code, it will broadcast it. When this transaction arrives to a 0.8.3 node, it will resend it truncated, and this will disconnect the 0.8.3 node from all Bitcoinj nodes surrounding it. I have not verified by testing that Bitcoinj does this, but I’m almost sure that the exception thrown will terminate the connection.
If the attack does not success the first time, the attacker can try as many times as he wants. As previously said, he can even use existing relayed transactions, expanding the input_count field. So after some minutes, and high number of modified txs, the attack must be successful.
The success probability depends on the number of old, 0.8.3 and bitcoinj nodes, and network topology. And the attackey may try sending the tx to different nodes to find the right path to the victim’s node. So I think that with enough tries, it will succeed even with very different client version mixes.

So basically the attack takes advantage of the network client mix, and remotely (an anonymously) harm 0.8.3/Bitcoinj nodes.
If the attacker knows that two victims are connected, one is a 0.8.3 nodes, and the other is a Bitcoinj node, he can remotely perform the attack to disconnect them.

Are these problems vulnerabilities of Bitcoin 0.8.3? It depends on what you expect from the protocol. If you expected the Satoshi code to be protected from DoS and 0-confirmation double-spends, then it is. It seems that Satoshi code was not designed to protect you from DoS, but it’s also true that the core developers have tried to add some DoS preventions since the Satoshi times.

In any case, it’s only a minor vulnerability or flaw.

Best regards, Sergio.

piotr_n
Legendary
*
Offline Offline

Activity: 2055
Merit: 1359


aka tonikt


View Profile WWW
July 19, 2013, 06:59:42 PM
 #2

Sorry, its a bit long to read, but if you don't mind I have a question already at the beginning.

Quote
Almost all objects that are exchanged between Satoshi clients are deserialized to be stored in memory and serialized again when they need to be forwarded. . Transactions are also first deserialized in order to be parsed, but they are stored “as is” in a temporary cache called MapRelay, and forwarded without being serialized again. I really don’t know why this is done. But the point is that the storage of messages as they are received leads to the possibility that an attacker creates messages much bigger than the actual transaction object contained, which, when stored in MapRelay memory, can be used to exhaust the memory of a node. This is the code that patches the vulnerability:

So, you are saying that checking whether a serialized size at the moment you receive the transaction, cutting if needed, and keeping the transaction as raw data is less DoS-resistant than keeping the transaction in objects and assembling it each time any node wants it?

The only thing I would change here is only banning those that send me too long txs.
But since we know that they might just route the shit, because of the bug, banning them would not be polite.
So its a hard moral problem Smiley

Check out gocoin - my original project of full bitcoin node & cold wallet written in Go.
PGP fingerprint: AB9E A551 E262 A87A 13BB  9059 1BE7 B545 CDF3 FD0E
Peter Todd
Legendary
*
expert
Offline Offline

Activity: 1120
Merit: 1160


View Profile
July 19, 2013, 07:02:22 PM
 #3

It looks like even having that cache enables and/or makes worse other types of attacks, so right now I'm thinking it probably should be removed completely, but I want to think more about the issue. (the extra memory usage is probably more expensive than re-serializing on demand)

Also yet another example of why zero-conf is dangerous.

piotr_n
Legendary
*
Offline Offline

Activity: 2055
Merit: 1359


aka tonikt


View Profile WWW
July 19, 2013, 07:05:03 PM
 #4

BTW, once again I want to raise my unheard voice, that txs with inputs that are not mined yet should be threat same as non-standard.
They don't make any economical sense for anyone, except saroshidice.
So unless you guys get a cut of it, or play there, please stop it Smiley

Check out gocoin - my original project of full bitcoin node & cold wallet written in Go.
PGP fingerprint: AB9E A551 E262 A87A 13BB  9059 1BE7 B545 CDF3 FD0E
Peter Todd
Legendary
*
expert
Offline Offline

Activity: 1120
Merit: 1160


View Profile
July 19, 2013, 07:24:54 PM
 #5

Quote from: Sergio_Demian_Lerner
It seems that Satoshi code was not designed to protect you from DoS, but it’s also true that the core developers have tried to add some DoS preventions since the Satoshi times. In any case, it’s only a minor vulnerability or flaw. What worries me is not that a bug was found, nor that a bug in the patch was found, but that the github commit of the patch does not show a history of  a discussion regarding the patch correctness, nor it is recorded if the code was audited and by whom. This lack of standardized protocol for the treatment of sensitive patches should be corrected, and I offer myself to review security critical code, whenever it is needed, and whenever I can.

FWIW the discussion regarding the patch happened in private email CC'd to all core developers and the bitcoinj developers; nearly everyone CC'd participated in the discussion, and did so in only a few hours. This patch was unusual because it was written during an active attack on mainnet, and doubly unusual because you only need a small number of patched nodes to stop the attack against the network as a whole. (individual nodes are still vulnerable)

It's fine to think about protocols for vetting security-related patches, but it'll be awhile before the Bitcoin project has the resources to hire the multiple full-time staff with overlapping schedules we need to really use those protocols effectively. Unfortunately it's an response time vs. risk tradeoff without one-size-fits-all answers.

Frankly the best I can say is donate to the Bitcoin foundation so they can hire more developers.

piotr_n
Legendary
*
Offline Offline

Activity: 2055
Merit: 1359


aka tonikt


View Profile WWW
July 19, 2013, 07:29:28 PM
 #6

So are you saying that he actually noticed the attack, and only drew your attention by proposing a stupid patch to prevent it?
Well, good for you, man. Smiley
Though, their solution, how to fix it, is better, IMO

Check out gocoin - my original project of full bitcoin node & cold wallet written in Go.
PGP fingerprint: AB9E A551 E262 A87A 13BB  9059 1BE7 B545 CDF3 FD0E
Sergio_Demian_Lerner (OP)
Hero Member
*****
expert
Offline Offline

Activity: 555
Merit: 654


View Profile WWW
July 19, 2013, 08:33:22 PM
 #7

My proposals to reduce this kinds of risks are:

- Make non-compact Var_ints non-standard (this should harm nobody, since I don't know why a client should store them in any other way that the encoding of minimum size)

- Remove the MapRelay functionality (or at least explain what exactly is the functionality!)

Pieter Wuille
Legendary
*
qt
Offline Offline

Activity: 1072
Merit: 1181


View Profile WWW
July 19, 2013, 10:47:55 PM
 #8

The reason for mapRelay (i.e., storing relayed data in wire format, rather than in parsed form), was I believe historically because Satoshi wanted to be able to extend the different data structures later in newer versions, and have old code relay them. I'm sure he didn't consider DoS attacks when designing this.

This may still be useful, and initially I was in favor of trying to keep this behaviour - there are certainly ideas for the future where this would be useful. However, each of those would require some form of authentication - a proof that the extension was in fact created by the creator of the transaction itself. The problem is that without having a definition for that extension, no old node can make this verification, and could easily be made to forward incorrect data with a transaction - potentially risking being considered a DoS attacker itself.

Given that, I think there is no safe way to use that feature anyway, and I'm in favor of just forwarding parsed data.

I do Bitcoin stuff.
Kouye
Sr. Member
****
Offline Offline

Activity: 336
Merit: 250


Cuddling, censored, unicorn-shaped troll.


View Profile
July 19, 2013, 11:17:39 PM
 #9

I'm not sure I understood all of this, sorry for being a newbie.
But are offchain Tx likely to be double spent too, using the same vin.size trick ?
Or does "unconfirmed transactions" already mean offchain, too ?  Lips sealed

[OVER] RIDDLES 2nd edition --- this was claimed. Look out for 3rd edition!
I won't ever ask for a loan nor offer any escrow service. If I do, please consider my account as hacked.
Peter Todd
Legendary
*
expert
Offline Offline

Activity: 1120
Merit: 1160


View Profile
July 20, 2013, 05:57:17 AM
 #10

The reason for mapRelay (i.e., storing relayed data in wire format, rather than in parsed form), was I believe historically because Satoshi wanted to be able to extend the different data structures later in newer versions, and have old code relay them. I'm sure he didn't consider DoS attacks when designing this.

FWIW I took a quick look at v0.1 - it has the comment "Save original serialized message so newer versions are preserved" in RelayMessage() so I'm sure your theory is correct.

Pages: [1]
  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!