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
This commit is contained in:
Etan Kissling 2021-12-11 15:46:15 +01:00
parent 9a1bb5e125
commit 45387ad4d2
No known key found for this signature in database
GPG Key ID: B21DA824C5A3D03D
2 changed files with 19 additions and 5 deletions

View File

@ -441,13 +441,17 @@ proc decodeMessagePacket(c: var Codec, fromAddr: Address, nonce: AESGCMNonce,
messageOpt: some(message), requestNonce: nonce, srcId: srcId)) messageOpt: some(message), requestNonce: nonce, srcId: srcId))
proc decodeWhoareyouPacket(c: var Codec, nonce: AESGCMNonce, proc decodeWhoareyouPacket(c: var Codec, nonce: AESGCMNonce,
iv, header: openArray[byte]): DecodeResult[Packet] = iv, header, ct: openArray[byte]): DecodeResult[Packet] =
# TODO improve this # TODO improve this
let authdata = header[staticHeaderSize..header.high()] let authdata = header[staticHeaderSize..header.high()]
# We now know the exact size that the authdata should be # We now know the exact size that the authdata should be
if authdata.len != idNonceSize + sizeof(uint64): if authdata.len != idNonceSize + sizeof(uint64):
return err("Invalid header length for whoareyou packet") 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 var idNonce: IdNonce
copyMem(addr idNonce[0], unsafeAddr authdata[0], idNonceSize) copyMem(addr idNonce[0], unsafeAddr authdata[0], idNonceSize)
let whoareyou = WhoareyouData(requestNonce: nonce, idNonce: idNonce, 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 # a valid message and thus we could increase here to the size of the smallest
# message possible. # message possible.
if ct.len < gcmTagSize: if ct.len < gcmTagSize:
return err("Invalid message length for ordinary message packet") return err("Invalid message length for handshake message packet")
let let
authdata = header[staticHeaderSize..header.high()] 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)) input.toOpenArray(ivSize + header.len, input.high))
of Whoareyou: of Whoareyou:
# Header size got checked in decode header
return decodeWhoareyouPacket(c, staticHeader.nonce, 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: of HandshakeMessage:
return decodeHandshakePacket(c, fromAddr, staticHeader.nonce, return decodeHandshakePacket(c, fromAddr, staticHeader.nonce,

View File

@ -288,7 +288,8 @@ suite "Discovery v5.1 Packet Encodings Test Vectors":
codecB.sessions.store(nodeA.id, nodeA.address.get(), codecB.sessions.store(nodeA.id, nodeA.address.get(),
hexToByteArray[aesKeySize](readKey), hexToByteArray[aesKeySize](dummyKey)) 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: check:
decoded.isOK() decoded.isOK()
decoded.get().messageOpt.isSome() decoded.get().messageOpt.isSome()
@ -318,6 +319,9 @@ suite "Discovery v5.1 Packet Encodings Test Vectors":
decoded.get().whoareyou.recordSeq == whoareyouEnrSeq decoded.get().whoareyou.recordSeq == whoareyouEnrSeq
decoded.get().whoareyou.challengeData == hexToSeqByte(whoareyouChallengeData) decoded.get().whoareyou.challengeData == hexToSeqByte(whoareyouChallengeData)
codecB.decodePacket(nodeA.address.get(),
hexToSeqByte(encodedPacket & "00")).isErr()
test "Ping Handshake Message Packet": test "Ping Handshake Message Packet":
const const
pingReqId = "0x00000001" pingReqId = "0x00000001"
@ -361,6 +365,9 @@ suite "Discovery v5.1 Packet Encodings Test Vectors":
decoded.get().message.ping.enrSeq == pingEnrSeq decoded.get().message.ping.enrSeq == pingEnrSeq
decoded.get().node.isNone() decoded.get().node.isNone()
codecB.decodePacket(nodeA.address.get(),
hexToSeqByte(encodedPacket & "00")).isErr()
test "Ping Handshake Message Packet with ENR": test "Ping Handshake Message Packet with ENR":
const const
pingReqId = "0x00000001" pingReqId = "0x00000001"
@ -408,6 +415,9 @@ suite "Discovery v5.1 Packet Encodings Test Vectors":
decoded.get().message.ping.enrSeq == pingEnrSeq decoded.get().message.ping.enrSeq == pingEnrSeq
decoded.get().node.isSome() decoded.get().node.isSome()
codecB.decodePacket(nodeA.address.get(),
hexToSeqByte(encodedPacket & "00")).isErr()
suite "Discovery v5.1 Additional Encode/Decode": suite "Discovery v5.1 Additional Encode/Decode":
test "Encryption/Decryption": test "Encryption/Decryption":
let let