fix(chain): cap block::Hash and CountedHeader preallocation at protocol-level constants#10570
Merged
mergify[bot] merged 1 commit intoMay 25, 2026
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
…ol-level constants `block::Hash::max_allocation` previously returned 65,535 (derived from `MAX_PROTOCOL_MESSAGE_LEN / 32`), and `CountedHeader::max_allocation` returned ~1,409 (same message-size-derived shape). Both are reachable via `Vec::zcash_deserialize` from any post-handshake peer through `getblocks`, `getheaders`, and `headers` messages. A peer can claim up to those counts and force `Vec::with_capacity` preallocation before any payload bytes are read. Cap at protocol-level constants: - `MAX_BLOCK_LOCATOR_LENGTH = 101`, a new `block`-module constant that matches Bitcoin Core's `MAX_LOCATOR_SZ` in `net_processing.cpp` (which zcashd inherits). - The existing `MAX_HEADERS_PER_MESSAGE = 160` constant from `crate::serialization::zcash_serialize`, already enforced on the sending side and at the codec level for `read_headers` (PR ZcashFoundation#10528). Same fix shape applied to `AddrV1`/`AddrV2` in PR ZcashFoundation#10494 for GHSA-xr93-pcq3-pxf8. CWE-770 (Allocation of Resources Without Limits or Throttling). Tests: - `block_hash_max_allocation` proptest updated to assert `Hash::max_allocation() == MAX_BLOCK_LOCATOR_LENGTH`. - `counted_header_max_allocation` proptest updated to assert `CountedHeader::max_allocation() == MAX_HEADERS_PER_MESSAGE as u64`. - Test-only constants `BLOCK_HEADER_LENGTH` and `MIN_COUNTED_HEADER_LEN`, which were used by the old message-size-derived formula in `header.rs`, moved into the `preallocate` test module where they are the only consumers.
Contributor
|
Thank you for your PR. We will take a look shortly. |
oxarbitrage
force-pushed
the
fix/block-hash-cap-and-counted-header
branch
from
May 25, 2026 11:12
5bbdec7 to
59a2f9c
Compare
oxarbitrage
approved these changes
May 25, 2026
oxarbitrage
left a comment
oxarbitrage
left a comment
Contributor
There was a problem hiding this comment.
Reviewed the full diff. The fix is correct and well-scoped:
- Hash::max_allocation: 65,535 to 101 (matches Bitcoin Core MAX_LOCATOR_SZ; legitimate locators are ~32 entries)
- CountedHeader::max_allocation: 1,409 to 160 (reuses existing MAX_HEADERS_PER_MESSAGE already enforced in the codec)
- Stacks correctly with #10563 generic 1024-element deserializer cap
- Both deserialization paths are live (read_getblocks, read_getheaders, read_headers)
- Tests updated correctly, unused constants moved to test module
Rebased onto main, resolved CHANGELOG conflict. All local checks pass (cargo test, clippy, fmt).
Contributor
Merge Queue Status
This pull request spent 11 minutes 55 seconds in the queue, including 35 seconds running CI. Required conditions to merge
|
This was referenced May 25, 2026
judah-caruso
pushed a commit
to ShieldedLabs/zebra-crosslink-staging
that referenced
this pull request
May 28, 2026
…ol-level constants (ZcashFoundation#10570) `block::Hash::max_allocation` previously returned 65,535 (derived from `MAX_PROTOCOL_MESSAGE_LEN / 32`), and `CountedHeader::max_allocation` returned ~1,409 (same message-size-derived shape). Both are reachable via `Vec::zcash_deserialize` from any post-handshake peer through `getblocks`, `getheaders`, and `headers` messages. A peer can claim up to those counts and force `Vec::with_capacity` preallocation before any payload bytes are read. Cap at protocol-level constants: - `MAX_BLOCK_LOCATOR_LENGTH = 101`, a new `block`-module constant that matches Bitcoin Core's `MAX_LOCATOR_SZ` in `net_processing.cpp` (which zcashd inherits). - The existing `MAX_HEADERS_PER_MESSAGE = 160` constant from `crate::serialization::zcash_serialize`, already enforced on the sending side and at the codec level for `read_headers` (PR ZcashFoundation#10528). Same fix shape applied to `AddrV1`/`AddrV2` in PR ZcashFoundation#10494 for GHSA-xr93-pcq3-pxf8. CWE-770 (Allocation of Resources Without Limits or Throttling). Tests: - `block_hash_max_allocation` proptest updated to assert `Hash::max_allocation() == MAX_BLOCK_LOCATOR_LENGTH`. - `counted_header_max_allocation` proptest updated to assert `CountedHeader::max_allocation() == MAX_HEADERS_PER_MESSAGE as u64`. - Test-only constants `BLOCK_HEADER_LENGTH` and `MIN_COUNTED_HEADER_LEN`, which were used by the old message-size-derived formula in `header.rs`, moved into the `preallocate` test module where they are the only consumers. Co-authored-by: dingledropper <dingledropper@users.noreply.github.com>
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
3 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.
Motivation
block::Hash::max_allocationpreviously returned 65,535 (derived fromMAX_PROTOCOL_MESSAGE_LEN / 32), andCountedHeader::max_allocationreturned ~1,409 (same message-size-derived shape). Both are reachable viaVec::zcash_deserializefrom any post-handshake peer throughgetblocks,getheaders, andheadersmessages. A peer can claim up to those counts and forceVec::with_capacitypreallocation before any payload bytes are read.The defense-in-depth fix at the deserializer level lands in PR #10563 (
Vec::with_capacity(external_count.min(1024))); this PR caps the per-type allocation to match the actual protocol-level ceilings, mirroring Bitcoin Core'sMAX_LOCATOR_SZfor locators and reusing Zebra's existingMAX_HEADERS_PER_MESSAGEfor headers.CWE-770 (Allocation of Resources Without Limits or Throttling).
Solution
Cap at protocol-level constants:
Locator (
block::Hash): introduceMAX_BLOCK_LOCATOR_LENGTH = 101inzebra-chain/src/block.rs, matching Bitcoin Core'sMAX_LOCATOR_SZinnet_processing.cpp(which zcashd inherits).block::Hash::max_allocation()now returns this constant.Headers (
CountedHeader): reuse the existingMAX_HEADERS_PER_MESSAGE = 160constant fromzebra-chain/src/serialization/zcash_serialize.rs, already enforced on the sending side and at the codec level forread_headers(PR fix(network): Enforce 160-entry cap inread_headers#10528).CountedHeader::max_allocation()now returnsMAX_HEADERS_PER_MESSAGE as u64. No new constant introduced — single source of truth across send/receive paths.Test cleanup:
BLOCK_HEADER_LENGTHandMIN_COUNTED_HEADER_LEN(used only by the old message-size-derivedmax_allocationformula inheader.rs) move into thepreallocatetest module where they are the only consumers. Avoidsdead_codewarnings under--features proptest-implwith-D warnings.Test evidence
cargo fmt --all -- --check✓cargo test -p zebra-chain --lib block::tests::preallocate✓ (4 tests)cargo clippy -p zebra-chain --all-targets -- -D warnings✓cargo clippy -p zebra-chain --all-targets --features proptest-impl -- -D warnings✓The
block_hash_max_allocationandcounted_header_max_allocationproptests are updated to assert the new constant-based caps:Hash::max_allocation() == MAX_BLOCK_LOCATOR_LENGTHCountedHeader::max_allocation() == MAX_HEADERS_PER_MESSAGE as u64Context
AddrV1/AddrV2per-type caps viaMAX_ADDRS_IN_MESSAGE.Vec::with_capacity(external_count.min(1024))at the sharedzcash_deserialize_external_countsite, so this per-type fix and the deserializer-level cap stack independently.MAX_HEADERS_PER_MESSAGE = 160is the long-standing Zcash convention from ZIP-204 and is already enforced atzebra-network/src/protocol/external/codec.rsforread_headers(PR fix(network): Enforce 160-entry cap inread_headers#10528, May 2026).CHANGELOG
Updated
CHANGELOG.mdunder[Unreleased]→ Security with the standard form referencing the protocol-level constants and the GHSA-xr93-pcq3-pxf8 sibling.AI disclosure
Patch and PR description drafted with assistance from Claude (Anthropic), following @mpguerra's email-level scope ("
MAX_BLOCK_LOCATOR_LENGTH = 101matching Bitcoin Core'sMAX_LOCATOR_SZ" and "CountedHeadercap as defense-in-depth"). I reviewed each step and am the responsible author.