#35936 net: reject oversized locators before allocating

full analysis

https://github.com/bitcoin/bitcoin/pull/35936 · l0rinc · +62/-20 in 4 files, 4 commits · labels: P2P

Goal

  • Disconnect peers that send oversized block locator counts before reading their payloads
  • Avoid unneeded memory allocations when parsing malformed getblocks and getheaders messages

Enforces `MAX_LOCATOR_SZ` during stream deserialization rather than after reading locator hashes for `getblocks` and `getheaders` messages. Adds a dedicated `LimitedVectorExceededError` exception to the `LIMITED_VECTOR` serialization formatter and introduces `CBlockLocator::LimitedRead` to detect oversized counts before allocating vector memory.

Problem: Peers sending truncated `getblocks` or `getheaders` payloads with advertised locator counts exceeding `MAX_LOCATOR_SZ` trigger generic deserialization errors before reaching the post-deserialization size check. Consequently, the node fails to disconnect the misbehaving peer as intended and may allocate memory before parsing fails.

Category: P2P (#15 of 65)

P3 · DoS protection

  • P3 because it fixes an edge-case bug where peers with truncated oversized locators escaped disconnection
  • It prevents lingering misbehaving connections and avoids unneeded memory allocations

P3 because it corrects an edge-case bug in P2P protocol enforcement where peers sending truncated oversized locators escaped disconnection. While not a severe vulnerability, fixing it prevents lingering misbehaving connections and avoids unneeded allocations.

Membership: Changes message parsing and peer disconnection logic for getblocks and getheaders in src/net_processing.cpp.

Factors: security/stability 1, bug 1, performance 1, user value 0, leverage 0

Category: Utilities (logging, arguments, libraries) (#20 of 66)

P3 · cleanup

  • P3 because it lets serialization utilities distinguish size limit violations from generic parse corruptions
  • It allows callers across the codebase to cleanly catch and handle vector limit breaches

P3 because it improves the serialization utility layer by differentiating size limit violations from generic deserialization corruptions, allowing call sites across the codebase to cleanly catch limit violations.

Membership: Extends LIMITED_VECTOR in src/serialize.h with a dedicated LimitedVectorExceededError exception class.

Factors: security/stability 1, bug 0, performance 0, user value 0, leverage 1

Reviewability: Ready

  • Ready for review as branch is clean and all reviewer feedback has been addressed

Branch is clean, CI passes, and all reviewer feedback has been incorporated.

Author status: Active; rebased and addressed all review feedback on 2026-09-04.

Resolved concerns:

  • jeanpablojp noted that counts above MAX_SIZE (33,554,432) threw generic std::ios_base::failure from ReadCompactSize range checks, bypassing the disconnect path; addressed in the September 4 force push by passing range_check=false and widening the size variable to uint64_t.
  • mzumsande and sedited questioned using exception control flow versus local stream parsing loops; ajtowns and l0rinc clarified that local manual loops would duplicate vector reading logic in net_processing, and no further objections were raised.

Agreement: Positive

  • Tested and approved after proposing a fix for counts above MAX_SIZE (jeanpablojp)
  • Approved the approach and implementation (achow101)
  • Endorsed the exception-based design over duplicating stream parsing loops (ajtowns)
  • Questioned parser consistency with headers without blocking (mzumsande, sedited)

Positive; prior ACKs from achow101 and jeanpablojp, with design rationale endorsed by ajtowns.

The approach received an Approach ACK and tACK from jeanpablojp, ACK from achow101, and co-authorship/rationale support from ajtowns. Questions regarding exception handling were answered without dispute.

  • jeanpablojp gave an Approach ACK and tACK, suggesting an extension for counts over MAX_SIZE which was adopted.
  • achow101 ACKed commit 725bf357d.
  • ajtowns defended the exception design against manual open-coded loops in net_processing.
  • mzumsande and sedited inquired about consistency with the HEADERS parser but did not block.

Review verdicts (DrahtBot): 0 (+2)

Files

26 lines under test/bench/ci.

  • src/net_processing.cpp +16/-14
  • src/test/net_tests.cpp +26/-0
  • src/serialize.h +12/-6
  • src/primitives/block.h +8/-0

Card

PR #35936 ensures that getblocks and getheaders messages reject and disconnect peers advertising locator counts over MAX_LOCATOR_SZ before allocating hashes, even if the payload is truncated. It introduces a dedicated LimitedVectorExceededError in serialize.h and adds CBlockLocator::LimitedRead to catch oversized counts during stream parsing. The change hardens p2p message handling against malformed inputs and prevents misbehaving peers from remaining connected. Review state is positive, with prior ACKs and testing from achow101 and jeanpablojp and rationale support from ajtowns.

Data

dossier JSON · extract JSON · model openrouter/google/gemini-3.8-flash, generated 2026-09-17T16:21, confidence high, input hash 24ba67e0db3441be