wire: use MaxProtocolMessageLength as a max size for MsgReject
What the developer wrote
Authored by Calvin Kim
wire: use MaxProtocolMessageLength as a max size for MsgReject
As we introduced a new MaxProtocolMessageLength, MsgReject max size
should also be lowered to MaxProtocolMessageLength.
If this isn't lowered and if we ever have a MsgReject message that is
bigger than MaxProtocolMessageLength, then the MaxPayloadLength() check
on the message will pass but it won't serialize in functions
WriteMessageWithEncodingN() and WriteV2MessageN() as both of these
functions have a separate check that each message isn't greater than
MaxProtocolMessageLength.
What changed, and why it matters
This commit tightens the maximum allowed size of Bitcoin protocol 'reject' messages so it matches the general message-size limit already used elsewhere. Before the fix, a reject message could claim a legal size under one check but then fail to send under a stricter second check, which could cause inconsistent handling between reading and writing network messages.
Treat as a low-to-moderate hardening fix. Review whether any other message types still use MaxMessagePayload where MaxProtocolMessageLength is now expected, and add regression tests covering the boundary between the two limits.
Security signals we found
Inconsistent size limits between message validation and serialization
Potential denial-of-service vector via oversized reject messages
Protocol wire-format hardening
Evidence from the diff
MsgReject.MaxPayloadLength() previously returned MaxMessagePayload (the legacy large limit). The project had introduced a smaller MaxProtocolMessageLength used by WriteMessageWithEncodingN and WriteV2MessageN. The mismatch meant a MsgReject whose payload was between MaxProtocolMessageLength and MaxMessagePayload would pass MaxPayloadLength validation but be rejected during serialization. The patch changes MaxPayloadLength to return MaxProtocolMessageLength for the latest protocol version, aligning the read-side and write-side limits.
Changed components
wire/msgreject.gowire/msgreject_test.goInspect captured patch +2 / −2
diff --git a/wire/msgreject.go b/wire/msgreject.go
index ea16dd1..7b7488f 100644
--- a/wire/msgreject.go
+++ b/wire/msgreject.go
@@ -175,7 +175,7 @@ func (msg *MsgReject) MaxPayloadLength(pver uint32) uint32 {
// Unfortunately the bitcoin protocol does not enforce a sane
// limit on the length of the reason, so the max payload is the
// overall maximum message payload.
- plen = MaxMessagePayload
+ plen = MaxProtocolMessageLength
}
return plen
diff --git a/wire/msgreject_test.go b/wire/msgreject_test.go
index 0285c50..7d9f952 100644
--- a/wire/msgreject_test.go
+++ b/wire/msgreject_test.go
@@ -77,7 +77,7 @@ func TestRejectLatest(t *testing.T) {
}
// Ensure max payload is expected value for latest protocol version.
- wantPayload := uint32(MaxMessagePayload)
+ wantPayload := uint32(MaxProtocolMessageLength)
maxPayload := msg.MaxPayloadLength(pver)
if maxPayload != wantPayload {
t.Errorf("MaxPayloadLength: wrong max payload length for "+
Why this scored 47/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.