p2p: check p2p state before parsing in handle_notify_new_transactions
What the developer wrote
Authored by selsta
p2p: check p2p state before parsing in handle_notify_new_transactions
What changed, and why it matters
This change moves a duplicate-transaction check so it only runs after the peer connection is in a normal, ready state. Before, a peer could send transaction data during protocol setup, and the node would parse and validate those transactions before confirming the connection was properly established. The patch prevents unnecessary processing of potentially malformed or duplicate transaction blobs during handshake phases.
Treat as a hardening/defensive fix. Review whether other P2P message handlers similarly parse peer input before state checks, and consider applying the same pattern consistently. No immediate emergency response is indicated by the diff alone.
Security signals we found
Reordering of validation/state checks to enforce state preconditions before parsing
Avoidance of parsing untrusted peer-supplied blobs before connection handshake completion
Potential denial-of-service reduction by not doing expensive tx parsing on non-normal peers
Evidence from the diff
In handle_notify_new_transactions, the code previously parsed and validated each transaction blob and checked for duplicates before verifying context.m_state == state_normal. The patch reorders these checks: it now returns early if the connection is not in state_normal, and only then parses/validates transactions and checks for duplicates. This avoids doing expensive or trust-requiring parsing work on connections that are not yet fully initialized. The duplicate detection and drop_connection behavior are preserved, just deferred until after state checks.
Changed components
src/cryptonote_protocol/cryptonote_protocol_handler.inlt_cryptonote_protocol_handler::handle_notify_new_transactionscryptonote_connection_context state handlingInspect captured patch +13 / −12
diff --git a/src/cryptonote_protocol/cryptonote_protocol_handler.inl b/src/cryptonote_protocol/cryptonote_protocol_handler.inl
index ae8db1e..8fa7f30 100644
--- a/src/cryptonote_protocol/cryptonote_protocol_handler.inl
+++ b/src/cryptonote_protocol/cryptonote_protocol_handler.inl
@@ -903,18 +903,6 @@ namespace cryptonote
int t_cryptonote_protocol_handler<t_core>::handle_notify_new_transactions(int command, NOTIFY_NEW_TRANSACTIONS::request& arg, cryptonote_connection_context& context)
{
MLOG_P2P_MESSAGE("Received NOTIFY_NEW_TRANSACTIONS (" << arg.txs.size() << " txes)");
- std::unordered_set<blobdata> seen;
- for (const auto &blob: arg.txs)
- {
- MLOGIF_P2P_MESSAGE(cryptonote::transaction tx; crypto::hash hash; bool ret = cryptonote::parse_and_validate_tx_from_blob(blob, tx, hash);, ret, "Including transaction " << hash);
- if (seen.find(blob) != seen.end())
- {
- LOG_PRINT_CCONTEXT_L1("Duplicate transaction in notification, dropping connection");
- drop_connection(context, false, false);
- return 1;
- }
- seen.insert(blob);
- }
if(context.m_state != cryptonote_connection_context::state_normal)
return 1;
@@ -928,6 +916,19 @@ namespace cryptonote
return 1;
}
+ std::unordered_set<blobdata> seen;
+ for (const auto &blob: arg.txs)
+ {
+ MLOGIF_P2P_MESSAGE(cryptonote::transaction tx; crypto::hash hash; bool ret = cryptonote::parse_and_validate_tx_from_blob(blob, tx, hash);, ret, "Including transaction " << hash);
+ if (seen.find(blob) != seen.end())
+ {
+ LOG_PRINT_CCONTEXT_L1("Duplicate transaction in notification, dropping connection");
+ drop_connection(context, false, false);
+ return 1;
+ }
+ seen.insert(blob);
+ }
+
/* If the txes were received over i2p/tor, the default is to "forward"
with a randomized delay to further enhance the "white noise" behavior,
potentially making it harder for ISP-level spies to determine which
Why this scored 38/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.