Merge bitcoin/bitcoin#35958: net: align v2 message type validation with v1 range
What the developer wrote
Authored by merge-script
Merge bitcoin/bitcoin#35958: net: align v2 message type validation with v1 range
cc577de954671378268bab2a78714869b4f7388d net: align v2 message type validation with v1 range (Bruno Garcia)
Pull request description:
BIP324 specifies the 13-byte long-form message type encoding as "an ASCII message type (as in the v1 P2P protocol)", but V2Transport::GetMessageType() accepted bytes up to 0x7F, while for V1 it only accepts printable ASCII (0x20-0x7E).
This changes V2 to match V1 on it and add test coverage.
ACKs for top commit:
nervana21:
tACK cc577de954671378268bab2a78714869b4f7388d
ajtowns:
utACK cc577de954671378268bab2a78714869b4f7388d
w0xlt:
ACK cc577de954671378268bab2a78714869b4f7388d
sedited:
ACK cc577de954671378268bab2a78714869b4f7388d
Tree-SHA512: 8c97ee20df2311949bbe9655c7e04507c4b47d3b18766aa6ae51691d0870f8a5c25ea54d74c9afb797754572d057b4240533da6bf3c2e0435f3cb32c5fb1c3af
What changed, and why it matters
This commit tightens the rules for what characters are allowed in message type names sent over Bitcoin's newer encrypted peer-to-peer (v2) transport. Previously, v2 allowed the byte 0x7F (the DEL control character), which is not allowed in the older v1 protocol. The change makes v2 match v1 by rejecting 0x7F and only accepting printable ASCII characters. It also adds tests to confirm this behavior. This is a consistency/security hardening fix, not an active exploit patch.
No immediate action required beyond normal review and testing. The change is a hardening fix. Nodes should ensure they are running a version that includes this alignment to avoid protocol ambiguity. Downstream implementers of BIP324 should verify their own message type validation matches v1 printable ASCII (0x20-0x7E).
Security signals we found
Protocol validation inconsistency between v1 and v2 P2P transports
BIP324 specification compliance fix
Input validation hardening for message type parsing
Potential for implementation divergence / fingerprinting / parsing ambiguity
Evidence from the diff
V2Transport::GetMessageType() in src/net.cpp previously accepted message type bytes in the range 0x20-0x7F, while the v1 protocol only accepts printable ASCII (0x20-0x7E). BIP324 specifies that v2 long-form message types should be ‘an ASCII message type (as in the v1 P2P protocol)’. The patch changes the upper bound from 0x7F to 0x7E, aligning v2 with v1. Tests are added in src/test/net_tests.cpp to verify that 0x7F is rejected and 0x7E is accepted.
Changed components
src/net.cpp: V2Transport::GetMessageType()src/test/net_tests.cpp: v2transport_testInspect captured patch +13 / −3
### src/net.cpp
@@ -1440,8 +1440,9 @@ std::optional<std::string> V2Transport::GetMessageType(std::span<const uint8_t>&
size_t msg_type_len{0};
while (msg_type_len < CMessageHeader::MESSAGE_TYPE_SIZE && contents[msg_type_len] != 0) {
- // Verify that message type bytes before the first 0x00 are in range.
- if (contents[msg_type_len] < ' ' || contents[msg_type_len] > 0x7F) {
+ // Verify that message type bytes before the first 0x00 are in range. BIP324 specifies the
+ // long message type encoding as "an ASCII message type (as in the v1 P2P protocol)".
+ if (contents[msg_type_len] < ' ' || contents[msg_type_len] > 0x7E) {
return {};
}
++msg_type_len;
### src/test/net_tests.cpp
@@ -1400,16 +1400,25 @@ BOOST_AUTO_TEST_CASE(v2transport_test)
tester.SendMessage(uint8_t(4), msg_data_1); // cmpctblock short id
tester.SendMessage(0, {}); // Invalidly encoded message
tester.SendMessage("tx", msg_data_2); // 12-character encoded message type
+ // Message type containing 0x7F (DEL): outside the printable-ASCII range
+ // accepted by the v1 transport, so must be rejected here as well.
+ tester.SendMessage(std::string{"t\x7f"}, msg_data_2);
+ // Message type containing 0x7E ('~'): at the boundary, still valid.
+ tester.SendMessage("t~", msg_data_2);
ret = tester.Interact();
BOOST_REQUIRE(ret);
- BOOST_REQUIRE(ret->size() == 3);
+ BOOST_REQUIRE(ret->size() == 5);
BOOST_REQUIRE((*ret)[0]);
BOOST_CHECK((*ret)[0]->m_type == "cmpctblock");
BOOST_CHECK(std::ranges::equal((*ret)[0]->m_recv, MakeByteSpan(msg_data_1)));
BOOST_CHECK(!(*ret)[1]);
BOOST_REQUIRE((*ret)[2]);
BOOST_CHECK((*ret)[2]->m_type == "tx");
BOOST_CHECK(std::ranges::equal((*ret)[2]->m_recv, MakeByteSpan(msg_data_2)));
+ BOOST_CHECK(!(*ret)[3]);
+ BOOST_REQUIRE((*ret)[4]);
+ BOOST_CHECK((*ret)[4]->m_type == "t~");
+ BOOST_CHECK(std::ranges::equal((*ret)[4]->m_recv, MakeByteSpan(msg_data_2)));
// Then send a message with a bit error, expecting failure. It's possible this failure does
// not occur immediately (when the length descriptor was modified), but it should comeWhy this scored 34/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.