fix(p2p): bound peer-supplied vector sizes in three NOTIFY handlers#194
Open
raw391 wants to merge 1 commit into
Open
fix(p2p): bound peer-supplied vector sizes in three NOTIFY handlers#194raw391 wants to merge 1 commit into
raw391 wants to merge 1 commit into
Conversation
handle_notify_new_master_node_vote, handle_notify_new_transactions and handle_request_fluffy_missing_tx iterate a peer-supplied vector without a size cap. The codebase already defines CURRENCY_PROTOCOL_MAX_OBJECT_REQUEST_COUNT and CURRENCY_PROTOCOL_MAX_TXS_REQUEST_COUNT for the same pattern in handle_request_get_blocks and handle_notify_request_get_txs. Adds CURRENCY_PROTOCOL_MAX_VOTES_COUNT and CURRENCY_PROTOCOL_MAX_FLASHES_COUNT, drops oversized messages with LOG_ERROR_CCONTEXT matching the existing sibling pattern.
raw391
force-pushed
the
fix/p2p-notify-vector-bounds
branch
from
e72939d to
c96063c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three NOTIFY handlers in
cryptonote_protocol_handler.inliterate a peer-supplied vector without checking its size first:handle_notify_new_master_node_voteat line 937 walksarg.votesdoing signature verification per elementhandle_notify_new_transactionsat line 1081 parsesarg.txsunderincoming_tx_lockhandle_request_fluffy_missing_txat line 980 does per-index dup check + block tx lookup overarg.missing_tx_indicesThe codebase already defines
CURRENCY_PROTOCOL_MAX_OBJECT_REQUEST_COUNTandCURRENCY_PROTOCOL_MAX_TXS_REQUEST_COUNTatcryptonote_protocol_handler.h:56-57and uses them inhandle_request_get_blocks(line 1190) andhandle_notify_request_get_txs(line 1737). This PR adds the same pattern to the three vote/tx/fluffy handlers, plusCURRENCY_PROTOCOL_MAX_FLASHES_COUNTfor the flash batch size inhandle_notify_new_transactions.Cap values: 1000 votes (50x current max quorum), 5000 txs and 500 missing-tx indices reuse existing constants, 1000 flashes matches the votes ceiling.
LOG_ERROR_CCONTEXTmatches the existing sibling pattern at lines 1190 and 1737.