bitpay / bitcore-p2p

Interface to the bitcoin P2P network for bitcore
MIT License
79 stars 275 forks source link

"Data still available after parsing" error crashes script #85

Open Flavien opened 8 years ago

Flavien commented 8 years ago

I use this simple code:

var newPool = new Pool({
    network: "testnet",
    maxSize: 100
});

newPool.connect();

However, after a minute or two, I get the following exception and the script crashes:

      throw new Error('Data still available after parsing');
            ^
Error: Data still available after parsing
    at Object.checkFinished (D:\btc-relay\node_modules\bitcore-p2p\lib\messages\utils.js:20:13)
    at AddrMessage.setPayload (D:\btc-relay\node_modules\bitcore-p2p\lib\messages\commands\addr.js:48:9)
    at Function.exported.add.exported.commands.(anonymous function).fromBuffer (D:\btc-relay\node_modules\bitcore-p2p\lib\messages\builder.js:75:15)
    at Messages._buildFromBuffer (D:\btc-relay\node_modules\bitcore-p2p\lib\messages\index.js:103:41)
    at Messages.parseBuffer (D:\btc-relay\node_modules\bitcore-p2p\lib\messages\index.js:74:15)
    at Peer._readMessage (D:\btc-relay\node_modules\bitcore-p2p\lib\peer.js:219:31)
    at Socket.<anonymous> (D:\btc-relay\node_modules\bitcore-p2p\lib\peer.js:167:10)
    at Socket.emit (events.js:107:17)
    at readableAddChunk (_stream_readable.js:163:16)
    at Socket.Readable.push (_stream_readable.js:126:10)
Press any key to continue...

How to handle the error and carry on?

braydonf commented 8 years ago

Interesting, is it bad data that is being sent? It would be useful to have a payload to see what is is, and incorporate into tests.

We may need to handle this check https://github.com/bitpay/bitcore-p2p/blob/master/lib/messages/commands/addr.js#L48 better.

Flavien commented 8 years ago

Here is the payload:

image

braydonf commented 8 years ago

Is this accurate?

> var buf = new Buffer(new Uint8Array([1,42,233,205,86,1,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,255,255,128,8,124,7,71,157,0,0,0,0]));
> buf.toString('hex');
'012ae9cd56010000000000000000000000000000000000ffff80087c07479d00000000'
Flavien commented 8 years ago

Yes that's correct.

braydonf commented 8 years ago

So it looks it's parsed with:

[ { services: <BN: 1>,
    ip: 
     { v6: '0000:0000:0000:0000:0000:ffff:8008:7c07',
       v4: '128.8.124.7' },
    port: 18333,
    time: Wed Feb 24 2016 12:32:26 GMT-0500 (EST) } ]

And has 4 bytes remaining (which may or may not be of concern).

Flavien commented 8 years ago

Well, it is of concern to Bitcore as it checks that the buffers has been completely read, and that causes Object.checkFinished to throw an exception.

Flavien commented 8 years ago

Any progress on fixing this? This is a serious blocker for us.

braydonf commented 8 years ago

It doesn't look like there is an issue parsing the info, as this matches up with https://en.bitcoin.it/wiki/Protocol_documentation#addr with the exception of the last four bytes:

01 - one addr
2ae9cd56 - time
0100000000000000 - services
00000000000000000000ffff80087c07 - ip address
479d - port
00000000 - ?
Flavien commented 8 years ago

Well, I don't know whether there is an issue parsing the info, but I do know that receiving an invalid message shouldn't crash the process. That's a pretty obvious DoS vulnerability.

braydonf commented 8 years ago

There are foreseeable other parsing and transaction/block validation issues, and connecting to a trusted peer is often used: https://github.com/bitpay/bitcore-p2p/blob/master/docs/pool.md#trusted-peers

Though we could certainly improve the robustness.

Flavien commented 8 years ago

Using trusted peers defeats the purpose of Bitcoin.

It shouldn't be very hard to add proper error handling so that when a parsing error occurs, the message is just ignored, instead of crashing the whole process.

digitalgoodsprovider commented 7 years ago

this makes my code crash also

Ark-Chen commented 5 years ago

I have encountered a similar problem, I still don't know how to solve it.https://github.com/bitpay/bitcore/issues/2183