Prysm: Prysm replies to malformed RANDOM PACKETS

[jrhea]

🐞 Bug Report

Description

Prysm doesn't properly validate RANDOM PACKET before replying with WHOAREYOU. Per the discv5 protocol a RANDOM_PACKET is sent to initiate a handshake and the receiving node replies with WHOAREYOU. The RANDOM_PACKET should have the following layout:

tag -> 32 bytes
auth-tag -> 12 bytes (RLP format)
random_data -> 44+ bytes

There are two issues

1) Prysm will reply to undersized RANDOM PACKETS.

The Discv5 rationale document specifically mentions that the RANDOM PACKET size should be > WHOAREYOU to ensure that an attack using the RANDOM PACKET format will require more bandwidth for the attacker. Prysm will reply to RANDOM PACKETS as small as 46 bytes - in other words, the random_data field does not seem to be required in this implementation. 88 bytes + 2 bytes for RLP length prefix should be the minimum acceptable size.

2) Prysm does not properly validate the RLP in the auth-tag field.

According to the RLP specification, this field should be prepended with 8c followed by 12 bytes.

The following is screenshot showing Prysm's reply to a malformed RANDOM PACKET that is undersized AND has invalid RLP encoding:

Invalid RANDOM PACKET:

Prysm's WHOAREYOU response:

For fun 🤥, I looked into the discovery implementation that Prysm uses at github.com/ethereum/go-ethereum/p2p/discover it looks like a packet is recieved, then decode() is called in then handlePacket() method of v5_udp.go. The next line contains this code:

    if err != nil {
        t.log.Debug("Bad discv5 packet", "id", fromID, "addr", addr, "err", err)
        return err
    }

A packet with a 0 byte payload will trigger the condition above with this message:

DEBUG[08-13|00:48:42.764] Bad discv5 packet                        id=677557726689c257 addr=192.168.1.100:2376  err="packet too short"

A packet with a 0 bytes < payload <= 32 bytes will trigger the condition above with this message:

DEBUG[08-13|00:50:12.904] Bad discv5 packet                        id=677557726689c257 addr=192.168.1.100:2376  err=EOF

A packet with a 33 bytes <= payload <= 45 bytes will work for a few seconds before it stops responding with a WHOAREYOU. However, other packets with a 45 bytes < payload < (65535 - headers) bytes will work indefinitely:

TRACE[08-13|00:39:52.0022] << UNKNOWN/v5                            id=677557726689c257 addr=192.168.1.100:5243
TRACE[08-13|00:39:52.0022] >> WHOAREYOU/v5                          id=677557726689c257 addr=192.168.1.100:5243

I believe what happens that the message is classified as unknownV5 here and ultimately results in the WHOAREYOU response sent here.

Disclaimer: I just eyeballed the code so a more thorough investigation is needed

🔬 Minimal Reproduction

You can recreate the issue by using this command to send a malformed message to Prysm:

> hping3 -d 46 --fast --udp -p [PORT] [IP]

[nisdas]

@jrhea Thanks for reporting this ! I will take a deeper look into this by this week.

[nisdas]

Seems like this is a known issue with the discovery V5 spec here:
https://github.com/ethereum/devp2p/pull/157

This should be fixed once v5.1 is finalized and all clients are updated to it.

[fjl]

Yes, it will be fixed in the v5.1 spec. I will implement it in go-ethereum, so you don't need to worry about it for Prysm.

[jrhea]

Seems like this is a known issue with the discovery V5 spec here:
ethereum/devp2p#157

This should be fixed once v5.1 is finalized and all clients are updated to it.

@nisdas sorry... I should have pinged you. I opened a similar issue in all the implementations and we worked it out in the Lighthouse issue. Sorry I didn't update you. 😔

[nisdas]

No worries @jrhea !

[jrhea]

Prysm does not properly validate the RLP in the auth-tag field.
According to the RLP specification, this field should be prepended with 8c followed by 12 bytes.

@nisdas, @fjl the other issue I found should probably be addressed as well. Before decoding the Auth_tag, the discovery code should verify that it contains the proper length prefix of 8c. bc this is discovery domain specific... it should be done outside of the RLP decoder. should I open an issue directly in the geth repo?

[nisdas]

@jrhea If I am understanding the issue correctly, shouldnt this be handled by the RLP decoder ? If we need the length prefix to be there for auth-tag according to the RLP spec

[jrhea]

If you look at the screenshot of the packet capture from Wireshark, you can see that a payload consisting of 46 (or more) bytes of 0x58 is decoded and replied to. This implies that whatever is decoding the auth_tag doesn’t care about the length prefix. Furthermore, the rlp library should only care about the RLP being valid so as long as the prefix matches the length. At the risk of being wrong, i went ahead and opened an issue to make sure it is looked at:

https://github.com/ethereum/go-ethereum/issues/21528

[fjl]

The auth_tag will not be RLP anymore in the v5.1 spec, so this issue will be fixed by the update as well.

[jrhea]

The auth_tag will not be RLP anymore in the v5.1 spec, so this issue will be fixed by the update as well.

it's a good thing we voted to update to 5.1 in the networking meeting this week 😅

[nisdas]

Ok that makes sense then @jrhea thanks for clarifying this. Seems like v5.1 will also take care of this then

[rauljordan]

https://github.com/prysmaticlabs/prysm/issues/7515 includes an update to discv5.1, so closing in favor of the tracking issue