Skip to content

fix: validate request ID length in Message::decode()#296

Merged
ackintosh merged 2 commits into
sigp:masterfrom
jsvisa:fix-large-requestId
Apr 18, 2026
Merged

fix: validate request ID length in Message::decode()#296
ackintosh merged 2 commits into
sigp:masterfrom
jsvisa:fix-large-requestId

Conversation

@jsvisa

@jsvisa jsvisa commented Feb 24, 2026

Copy link
Copy Markdown
Contributor

Description

Message::decode() was constructing RequestId directly from decoded bytes without validating the length. The discv5 spec limits request IDs to a maximum of 8 bytes:

The first element of every message-data list is the request ID. request-id is an RLP byte array of length <= 8 bytes. For requests, this value is assigned by the requester. The recipient of a message must mirror the value in the request-id element of the response. The selection of appropriate values for request IDs is left to the implementation.

This caused the node to accept and echo back oversized request IDs (e.g. 9 bytes) in PONG responses, which violates the spec. The node should silently drop messages with invalid request IDs.

Fixes the hive devp2p PingLargeRequestID conformance test.

Notes & open questions

Change checklist

  • Self-review
  • Documentation updates if relevant
  • Tests if relevant

jsvisa and others added 2 commits February 24, 2026 16:08
Message::decode() was constructing RequestId directly from decoded bytes
without validating the length. The discv5 spec limits request IDs to a
maximum of 8 bytes, and RequestId::decode() already enforces this, but
it was not being called during message decoding.

This caused the node to accept and echo back oversized request IDs
(e.g. 9 bytes) in PONG responses, which violates the spec. The node
should silently drop messages with invalid request IDs.

Fixes the hive devp2p PingLargeRequestID conformance test.

Signed-off-by: Delweng <delweng@gmail.com>

@jxs jxs left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, and thanks for this! LGTM

@jxs jxs requested a review from ackintosh April 16, 2026 09:13

@ackintosh ackintosh left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@ackintosh ackintosh merged commit c0a3c63 into sigp:master Apr 18, 2026
7 checks passed
@jsvisa jsvisa deleted the fix-large-requestId branch April 19, 2026 00:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants