fix(network): Enforce 160-entry cap in read_headers#10528
Merged
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
upbqdn
force-pushed
the
fix/ghsa-rgwx-8r98-p34c
branch
from
May 1, 2026 09:53
23ca0b1 to
3c5a047
Compare
upbqdn
force-pushed
the
fix/read-headers-protocol-cap
branch
from
May 1, 2026 09:53
82d3051 to
af1ce94
Compare
mpguerra
previously approved these changes
May 1, 2026
upbqdn
changed the title
read_headers - GHSA-438q-jx8f-cccvread_headers
upbqdn
force-pushed
the
fix/ghsa-rgwx-8r98-p34c
branch
from
May 1, 2026 13:15
3c5a047 to
5f5b8d0
Compare
upbqdn
force-pushed
the
fix/read-headers-protocol-cap
branch
from
May 1, 2026 13:15
af1ce94 to
d2dcc2b
Compare
upbqdn
force-pushed
the
fix/ghsa-rgwx-8r98-p34c
branch
from
May 1, 2026 15:25
5f5b8d0 to
0ad6ff6
Compare
upbqdn
force-pushed
the
fix/read-headers-protocol-cap
branch
from
May 1, 2026 15:25
d2dcc2b to
5d0b956
Compare
conradoplg
force-pushed
the
fix/ghsa-rgwx-8r98-p34c
branch
from
May 1, 2026 17:04
0ad6ff6 to
397ab72
Compare
…on bypass Add a test proving that the codec accepts 161 headers in an inbound `headers` message, exceeding the Zcash protocol cap of 160. The receive path relies on TrustedPreallocate (ceiling ≈ 1 409) instead of enforcing MAX_FIND_BLOCK_HEADERS_RESULTS.
…8q-jx8f-cccv) read_headers() relied on TrustedPreallocate which allows up to 1,409 CountedHeader entries per message — ~8.8x the Zcash protocol limit of 160. Unauthenticated peers could force excessive preallocation and parse work by sending oversized headers messages. Deserialize the CompactSize count explicitly and reject messages exceeding MAX_HEADERS_PER_MESSAGE (160) before parsing any entries. The new constant lives in zebra-chain so both zebra-network (receive) and zebra-state (send) can reference the same protocol value.
conradoplg
force-pushed
the
fix/read-headers-protocol-cap
branch
from
May 1, 2026 17:06
5d0b956 to
81f80c7
Compare
oxarbitrage
pushed a commit
to dingledropper/zebra
that referenced
this pull request
May 25, 2026
…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.
mergify Bot
pushed a commit
that referenced
this pull request
May 25, 2026
…ol-level constants (#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 #10528). Same fix shape applied to `AddrV1`/`AddrV2` in PR #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>
judah-caruso
pushed a commit
to ShieldedLabs/zebra-crosslink-staging
that referenced
this pull request
May 28, 2026
…n#10528) * test(network): reproduce GHSA-438q-jx8f-cccv read_headers preallocation bypass Add a test proving that the codec accepts 161 headers in an inbound `headers` message, exceeding the Zcash protocol cap of 160. The receive path relies on TrustedPreallocate (ceiling ≈ 1 409) instead of enforcing MAX_FIND_BLOCK_HEADERS_RESULTS. * fix(network): enforce 160-entry protocol cap in read_headers (GHSA-438q-jx8f-cccv) read_headers() relied on TrustedPreallocate which allows up to 1,409 CountedHeader entries per message — ~8.8x the Zcash protocol limit of 160. Unauthenticated peers could force excessive preallocation and parse work by sending oversized headers messages. Deserialize the CompactSize count explicitly and reject messages exceeding MAX_HEADERS_PER_MESSAGE (160) before parsing any entries. The new constant lives in zebra-chain so both zebra-network (receive) and zebra-state (send) can reference the same protocol value. --------- Co-authored-by: Alfredo Garcia <oxarbitrage@gmail.com>
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
4 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.
Reproduce and propose fix.