wire: enforce full payload consumption in ReadMessage
What the developer wrote
Authored by Olaoluwa Osuntokun
wire: enforce full payload consumption in ReadMessage
In this commit, we add a strictness check to
readMessageWithEncodingNInternal that verifies all payload bytes were
consumed by BtcDecode. After BtcDecode returns, the remaining length of
the payload buffer is checked, and if any bytes remain unconsumed, the
message is rejected with a MessageError.
Previously, the payload buffer was passed to BtcDecode without any
post-decode validation, meaning any bytes beyond what BtcDecode consumed
were silently ignored. This check ensures wire message payloads are
exactly the size needed for the decoded message, tightening our
conformance to the expected wire format.
What changed, and why it matters
This change tightens how btcd reads network messages from other Bitcoin peers. Previously, a peer could tack extra bytes onto the end of a valid message and btcd would silently accept it. Now those messages are rejected. The commit message notes that such trailing bytes could have been stored, for example in the block database, so this closes a potential avenue for injecting unwanted data or causing confusion.
Treat as a security-hardening fix and include in release notes. Users running nodes should upgrade to avoid accepting malformed wire messages with trailing bytes. Review whether any existing stored data may have included such trailing bytes, though the commit does not indicate active exploitation.
Security signals we found
Strict input validation added after decode step
Trailing payload bytes now rejected instead of silently ignored
Commit message explicitly mentions peer could append arbitrary trailing bytes and that data could be persisted
Potential data-integrity / stored-payload concern in block database mentioned by commit author
Evidence from the diff
The patch adds a post-decode check in readMessageWithEncodingNInternal in wire/message.go. After BtcDecode decodes a message from a fixed-size payload reader pr, it now checks pr.Len() > 0. If any payload bytes remain unconsumed, it returns a MessageError. This enforces that wire message payloads are exactly the size required by the decoded message, preventing trailing-byte acceptance.
Changed components
wire/message.goreadMessageWithEncodingNInternalReadMessage / network message decoding pathInspect captured patch +10 / −0
diff --git a/wire/message.go b/wire/message.go
index 39c1dd0..72e93bb 100644
--- a/wire/message.go
+++ b/wire/message.go
@@ -669,6 +669,16 @@ func readMessageWithEncodingNInternal(r io.Reader, pver uint32,
return totalBytes, nil, nil, err
}
+ // Reject messages where the payload was not fully consumed by
+ // BtcDecode. A peer could otherwise append arbitrary trailing bytes
+ // to an otherwise valid message, which would be silently accepted
+ // and persisted (e.g., in the block database).
+ if pr.Len() > 0 {
+ str := fmt.Sprintf("message payload has %d extra bytes "+
+ "after decode", pr.Len())
+ return totalBytes, nil, nil, messageError("ReadMessage", str)
+ }
+
return totalBytes, msg, payload, nil
}
Why this scored 64/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.