status-im/nim-eth-p2p

Avoid trivial denial-of-service opportunities

arnetheduck opened this issue · 1 comments

There's plenty of code like the following, that every time creates a security problem due to how one can easily stuff the client with silent peers, as a form of cheap DoS - without a timeout they will stay in limbo indefinitely:

await transport.readExactly(addr authMsg[0], len(authMsg))

Jacek Sieka
@arnetheduck
18:11

I'm looking at the peer code a bit:

    await transport.readExactly(addr authMsg[0], len(authMsg))

if the peer doesn't send anything, what will happen?
(this is from the incoming-peer handler,rlpxAccept
Eugene Kabanov
@cheatfate
18:15
@arnetheduck this depends, if peer doesn't send anything but not disconnect then this function will stuck, if it disconnects and number of bytes transferred will be less then len(authMsg) TransportIncompleteError exception will be raised, in rare cases TransportOsError can be raised.
kdeme commented

Yes, I've seen similar issues in even more obvious places: like here and also here.
It's also in the documentation: https://github.com/status-im/nim-eth-p2p/blob/master/README.md#implementing-handshakes-and-reacting-to-other-events

I had initially implemented it like that in Whisper, but then temporary fixed like this, until zah added the handshake.

Should be altered in eth, les and documentation also.