From 45387ad4d2cb6809f3fbc898029f9931a20c29d9 Mon Sep 17 00:00:00 2001
From: Etan Kissling <etan@status.im>
Date: Sat, 11 Dec 2021 15:46:15 +0100
Subject: [PATCH] reject WHOAREYOU packets with non-empty message

This changes the `discv5` parser to reject malformed WHOAREYOU packets
that have a non-0 message length. The extra data used to be ignored.
The `message` part of WHOAREYOU packets is always empty.
See https://github.com/ethereum/devp2p/blob/master/discv5/discv5-wire.md
---
 eth/p2p/discoveryv5/encoding.nim        | 12 ++++++++----
 tests/p2p/test_discoveryv5_encoding.nim | 12 +++++++++++-
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/eth/p2p/discoveryv5/encoding.nim b/eth/p2p/discoveryv5/encoding.nim
index 0f4e2e6..7b33817 100644
--- a/eth/p2p/discoveryv5/encoding.nim
+++ b/eth/p2p/discoveryv5/encoding.nim
@@ -441,13 +441,17 @@ proc decodeMessagePacket(c: var Codec, fromAddr: Address, nonce: AESGCMNonce,
     messageOpt: some(message), requestNonce: nonce, srcId: srcId))
 
 proc decodeWhoareyouPacket(c: var Codec, nonce: AESGCMNonce,
-    iv, header: openArray[byte]): DecodeResult[Packet] =
+    iv, header, ct: openArray[byte]): DecodeResult[Packet] =
   # TODO improve this
   let authdata = header[staticHeaderSize..header.high()]
   # We now know the exact size that the authdata should be
   if authdata.len != idNonceSize + sizeof(uint64):
     return err("Invalid header length for whoareyou packet")
 
+  # The `message` part of WHOAREYOU packets is always empty.
+  if ct.len != 0:
+    return err("Invalid message length for whoareyou packet")
+
   var idNonce: IdNonce
   copyMem(addr idNonce[0], unsafeAddr authdata[0], idNonceSize)
   let whoareyou = WhoareyouData(requestNonce: nonce, idNonce: idNonce,
@@ -468,7 +472,7 @@ proc decodeHandshakePacket(c: var Codec, fromAddr: Address, nonce: AESGCMNonce,
   # a valid message and thus we could increase here to the size of the smallest
   # message possible.
   if ct.len < gcmTagSize:
-    return err("Invalid message length for ordinary message packet")
+    return err("Invalid message length for handshake message packet")
 
   let
     authdata = header[staticHeaderSize..header.high()]
@@ -580,9 +584,9 @@ proc decodePacket*(c: var Codec, fromAddr: Address, input: openArray[byte]):
       input.toOpenArray(ivSize + header.len, input.high))
 
   of Whoareyou:
-    # Header size got checked in decode header
     return decodeWhoareyouPacket(c, staticHeader.nonce,
-      input.toOpenArray(0, ivSize - 1), header)
+      input.toOpenArray(0, ivSize - 1), header,
+      input.toOpenArray(ivSize + header.len, input.high))
 
   of HandshakeMessage:
     return decodeHandshakePacket(c, fromAddr, staticHeader.nonce,
diff --git a/tests/p2p/test_discoveryv5_encoding.nim b/tests/p2p/test_discoveryv5_encoding.nim
index f93ee5e..29619b4 100644
--- a/tests/p2p/test_discoveryv5_encoding.nim
+++ b/tests/p2p/test_discoveryv5_encoding.nim
@@ -288,7 +288,8 @@ suite "Discovery v5.1 Packet Encodings Test Vectors":
     codecB.sessions.store(nodeA.id, nodeA.address.get(),
       hexToByteArray[aesKeySize](readKey), hexToByteArray[aesKeySize](dummyKey))
 
-    let decoded = codecB.decodePacket(nodeA.address.get(), hexToSeqByte(encodedPacket))
+    let decoded = codecB.decodePacket(nodeA.address.get(),
+      hexToSeqByte(encodedPacket))
     check:
       decoded.isOK()
       decoded.get().messageOpt.isSome()
@@ -318,6 +319,9 @@ suite "Discovery v5.1 Packet Encodings Test Vectors":
       decoded.get().whoareyou.recordSeq == whoareyouEnrSeq
       decoded.get().whoareyou.challengeData == hexToSeqByte(whoareyouChallengeData)
 
+      codecB.decodePacket(nodeA.address.get(),
+        hexToSeqByte(encodedPacket & "00")).isErr()
+
   test "Ping Handshake Message Packet":
     const
       pingReqId = "0x00000001"
@@ -361,6 +365,9 @@ suite "Discovery v5.1 Packet Encodings Test Vectors":
       decoded.get().message.ping.enrSeq == pingEnrSeq
       decoded.get().node.isNone()
 
+      codecB.decodePacket(nodeA.address.get(),
+        hexToSeqByte(encodedPacket & "00")).isErr()
+
   test "Ping Handshake Message Packet with ENR":
     const
       pingReqId = "0x00000001"
@@ -408,6 +415,9 @@ suite "Discovery v5.1 Packet Encodings Test Vectors":
       decoded.get().message.ping.enrSeq == pingEnrSeq
       decoded.get().node.isSome()
 
+      codecB.decodePacket(nodeA.address.get(),
+        hexToSeqByte(encodedPacket & "00")).isErr()
+
 suite "Discovery v5.1 Additional Encode/Decode":
   test "Encryption/Decryption":
     let