highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-11-12 15:09:59 +01:00
Code Issues Packages Projects Releases Wiki Activity

net: Sanitize message type for logging

Browse Source 
- 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.
This commit is contained in:
W. J. van der Laan
2021-05-06 17:25:17 +02:00
parent 9c05da4a5c
commit 955eee7680
2 changed files with 5 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
src/net.cpp
View File

@@ -687,13 +687,13 @@ int V1TransportDeserializer::readHeader(Span<const uint8_t> msg_bytes)
// Check start string, network magic // Check start string, network magic
if (memcmp(hdr.pchMessageStart, m_chain_params.MessageStart(), CMessageHeader::MESSAGE_START_SIZE) != 0) { 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; return -1;
} }
// reject messages larger than MAX_SIZE or MAX_PROTOCOL_MESSAGE_LENGTH // reject messages larger than MAX_SIZE or MAX_PROTOCOL_MESSAGE_LENGTH
if (hdr.nMessageSize > MAX_SIZE || hdr.nMessageSize > 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; return -1;
} }
@@ -755,7 +755,7 @@ std::optional<CNetMessage> V1TransportDeserializer::GetMessage(const std::chrono
msg = std::nullopt; msg = std::nullopt;
} else if (!hdr.IsCommandValid()) { } else if (!hdr.IsCommandValid()) {
LogPrint(BCLog::NET, "HEADER ERROR - COMMAND (%s, %u bytes), peer=%d\n", 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; out_err_raw_size = msg->m_raw_message_size;
msg.reset(); msg.reset();
} }

4
test/functional/p2p_invalid_messages.py
View File

@@ -37,7 +37,7 @@ VALID_DATA_LIMIT = MAX_PROTOCOL_MESSAGE_LENGTH - 5 # Account for the 5-byte len
class msg_unrecognized: class msg_unrecognized:
"""Nonsensical message. Modeled after similar types in test_framework.messages.""" """Nonsensical message. Modeled after similar types in test_framework.messages."""
msgtype = b'badmsg' msgtype = b'badmsg\x01'
def __init__(self, *, str_data): def __init__(self, *, str_data):
self.str_data = str_data.encode() if not isinstance(str_data, bytes) else 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): def test_magic_bytes(self):
self.log.info("Test message with invalid magic bytes disconnects peer") self.log.info("Test message with invalid magic bytes disconnects peer")
conn = self.nodes[0].add_p2p_connection(P2PDataStore()) 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")) msg = conn.build_message(msg_unrecognized(str_data="d"))
# modify magic bytes # modify magic bytes
msg = b'\xff' * 4 + msg[4:] msg = b'\xff' * 4 + msg[4:]