diff options
author | W. J. van der Laan <laanwj@protonmail.com> | 2021-05-06 17:25:17 +0200 |
---|---|---|
committer | W. J. van der Laan <laanwj@protonmail.com> | 2021-05-06 17:30:52 +0200 |
commit | 955eee76803c098978cf0bbc7f1f6d3c230544e2 (patch) | |
tree | 43972207a367ea0b51cd416ff94ab9c315a24071 | |
parent | 9c05da4a5c57d7fd0fc53817ad6f0a07c0f08b5b (diff) | |
download | bitcoin-955eee76803c098978cf0bbc7f1f6d3c230544e2.tar.xz |
net: Sanitize message type for logging
- Use `SanitizeString` when logging message errors to make sure that the
message type is sanitized.
- For the `MESSAGESTART` error don't inspect and log header details at
all: receiving invalid start bytes makes it likely that the packet isn't
even formatted as valid P2P message. Logging the four unexpected start
bytes should be enough.
- Update `p2p_invalid_messages.py` test to check this.
Issue reported by gmaxwell.
-rw-r--r-- | src/net.cpp | 6 | ||||
-rwxr-xr-x | test/functional/p2p_invalid_messages.py | 4 |
2 files changed, 5 insertions, 5 deletions
diff --git a/src/net.cpp b/src/net.cpp index f55d3e2418..02e3672493 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -687,13 +687,13 @@ int V1TransportDeserializer::readHeader(Span<const uint8_t> msg_bytes) // Check start string, network magic if (memcmp(hdr.pchMessageStart, m_chain_params.MessageStart(), CMessageHeader::MESSAGE_START_SIZE) != 0) { - LogPrint(BCLog::NET, "HEADER ERROR - MESSAGESTART (%s, %u bytes), received %s, peer=%d\n", hdr.GetCommand(), hdr.nMessageSize, HexStr(hdr.pchMessageStart), m_node_id); + LogPrint(BCLog::NET, "HEADER ERROR - MESSAGESTART received %s, peer=%d\n", HexStr(hdr.pchMessageStart), m_node_id); return -1; } // reject messages larger than MAX_SIZE or MAX_PROTOCOL_MESSAGE_LENGTH if (hdr.nMessageSize > MAX_SIZE || hdr.nMessageSize > MAX_PROTOCOL_MESSAGE_LENGTH) { - LogPrint(BCLog::NET, "HEADER ERROR - SIZE (%s, %u bytes), peer=%d\n", hdr.GetCommand(), hdr.nMessageSize, m_node_id); + LogPrint(BCLog::NET, "HEADER ERROR - SIZE (%s, %u bytes), peer=%d\n", SanitizeString(hdr.GetCommand()), hdr.nMessageSize, m_node_id); return -1; } @@ -755,7 +755,7 @@ std::optional<CNetMessage> V1TransportDeserializer::GetMessage(const std::chrono msg = std::nullopt; } else if (!hdr.IsCommandValid()) { LogPrint(BCLog::NET, "HEADER ERROR - COMMAND (%s, %u bytes), peer=%d\n", - hdr.GetCommand(), msg->m_message_size, m_node_id); + SanitizeString(hdr.GetCommand()), msg->m_message_size, m_node_id); out_err_raw_size = msg->m_raw_message_size; msg.reset(); } diff --git a/test/functional/p2p_invalid_messages.py b/test/functional/p2p_invalid_messages.py index c0b3c2cb12..c81886a982 100755 --- a/test/functional/p2p_invalid_messages.py +++ b/test/functional/p2p_invalid_messages.py @@ -37,7 +37,7 @@ VALID_DATA_LIMIT = MAX_PROTOCOL_MESSAGE_LENGTH - 5 # Account for the 5-byte len class msg_unrecognized: """Nonsensical message. Modeled after similar types in test_framework.messages.""" - msgtype = b'badmsg' + msgtype = b'badmsg\x01' def __init__(self, *, str_data): self.str_data = str_data.encode() if not isinstance(str_data, bytes) else str_data @@ -104,7 +104,7 @@ class InvalidMessagesTest(BitcoinTestFramework): def test_magic_bytes(self): self.log.info("Test message with invalid magic bytes disconnects peer") conn = self.nodes[0].add_p2p_connection(P2PDataStore()) - with self.nodes[0].assert_debug_log(['HEADER ERROR - MESSAGESTART (badmsg, 2 bytes), received ffffffff']): + with self.nodes[0].assert_debug_log(['HEADER ERROR - MESSAGESTART received ffffffff']): msg = conn.build_message(msg_unrecognized(str_data="d")) # modify magic bytes msg = b'\xff' * 4 + msg[4:] |