ethereum / go-ethereum

Go implementation of the Ethereum protocol
https://geth.ethereum.org
GNU Lesser General Public License v3.0
47.55k stars 20.13k forks source link

Spec non compliance issue on the Ping Packet of Rlpx #28169

Open mohasdev opened 1 year ago

mohasdev commented 1 year ago

System information

Geth version: Geth/v1.13.1-unstable-16cd1a75 CL client : lighthouse OS & Version: Linux

Describe the bug

There is 1 issue on the ping packet from rlpx when Geth don't respond to a ping, the issues are spec non compliance issues .

According to the devp2p specs, if a node send a ping with a wrong version field in the Ping packet node should respond .

" Implementations must ignore any mismatches in auth-vsn and ack-vsn "

If i send a rlpx ping packet with the string : >=&':7!/+#4 in the version field , Geth don't respond.

Steps to reproduce

You can use the implementation of your choice and send a ping message to a Geth node, or using the devp2p binaries from geth and edit the ping message .

Or you can use D4C, a modified version of geth that send fuzzed message , just clone the repo and use this command (you need to have go in your computer) : make all

then use the corresponding commands that send a ping message :

./build/bin/devp2p rlpx wrong-version-ping < enode address > random-fuzzer 1

Ressources

The expected behaviors come from :

Devp2p specifications

jsvisa commented 1 year ago

Implementations must ignore any mismatches in auth-vsn and ack-vsn If i send a rlpx ping packet with the string : >=&':7!/+https://github.com/ethereum/go-ethereum/pull/4 in the version field , Geth don't respond.

So, in my opinion, this is spec-compliant, it's correct.

mohasdev commented 1 year ago

Implementations must ignore any mismatches in auth-vsn and ack-vsn If i send a rlpx ping packet with the string : >=&':7!/+#4 in the version field , Geth don't respond.

So, in my opinion, this is spec-compliant, it's correct.

When i say Geth don't respond i'm talking about the ping so geth is not ignoring the mismatch . If the mismatch was ignored , geth should respond . And according to the specs : Implementations must ignore any mismatches in auth-vsn and ack-vsn

mohasdev commented 1 year ago

Another issue on the same packet :

According to the devp2p specs : "Implementations must also ignore any additional list elements in auth-body and ack-body." if a node send a rlpx ping with extra data fields in auth-body node should ignore this.

If i send a ping packet with two additional fields who contains the string ?9166@+"6,=!(# ,Geth don't respond and i get an EOF.