Skip to content

fix(sync): validate discovery hash responses#151

Merged
p0mvn merged 2 commits into
security-portsfrom
security-port/sync-discovery-validation
Jul 14, 2026
Merged

fix(sync): validate discovery hash responses#151
p0mvn merged 2 commits into
security-portsfrom
security-port/sync-discovery-validation

Conversation

@p0mvn

@p0mvn p0mvn commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Motivation

This fixes a DoS variant of GHSA-h9hm-m2xj-4rq9 where malformed nonempty FindBlocks responses could poison sync state. For example, a response shaped like [unknown, known, random] could cause the node to enqueue known hashes and then cancel future downloads when the known block download failed.

This PR cleanly ports the original Zebra fix onto Zakura's security-ports branch.

Solution

  • Validate legacy discovery hash responses before mutating download state.
  • Reject duplicate hashes, known hashes after the first unknown hash, locator echoes in advertised suffixes, and nonproductive shapes such as [R].
  • Apply validation in both tip discovery and extension-building paths.
  • Add regression coverage for malformed discovery and extension responses.
  • Update Zakura's newer checkpoint-refill regression test for the additional validation state queries introduced by the port.

Tests

  • cargo fmt --all -- --check
  • cargo clippy --workspace --all-targets -- -D warnings
  • cargo test -p zakura components::sync (59 passed)
  • IDE diagnostics: no errors

Specifications & References

Follow-up Work

None.

AI Disclosure

  • No AI tools were used in this PR
  • AI tools were used: Cursor GPT-5.6 Sol ported the original commit, reconciled its validation requests with Zakura's checkpoint-refill regression test, and ran verification.

PR Checklist

  • The PR title follows conventional commits format: type(scope): description.
  • The PR follows the contribution guidelines.
  • This change was previously discussed and approved in the original PR linked above.
  • The solution is tested.
  • Documentation and changelogs are up to date; no additional changes are needed for this direct security port.

@v12-auditor

v12-auditor Bot commented Jul 14, 2026

Copy link
Copy Markdown

Note

Complete: Audit complete. V12 found two issues worth reviewing.

Open the full results here.

FindingSeverityDetails
F-98770 🟠 High
Invalid responses clear stall counters

The new FindBlocks validation rejects non-empty peer responses locally, but peer accountability still treats every non-empty BlockHashes response as successful. A malicious peer can return hashes containing a duplicate or a known hash after the first unknown; first_unknown_in_find_blocks_response returns Ok(None) and obtain_tips skips that response without adding any hashes to the download set. The peer-set wrapper has already classified the same non-empty response as StallOutcome::Clear, and drain_stall_events clears that peer's stall counter instead of disconnecting it. If the peers selected by the three-way fanout are malicious or the node is eclipsed, each obtain round can produce no usable tips while the existing FindBlocks stall protection is bypassed.

F-98771 🟠 High
Oversized responses force state lookups

The new validation paths perform a KnownBlock state-service lookup for attacker-controlled hashes without first enforcing the syncer's 500-hash expectation. In obtain_tips, first_unknown_in_find_blocks_response checks the state for each hash until the first unknown and then for every later hash to reject known suffixes. In build_extend, the diff clones the state service into the background extension future and validate_extend_find_blocks_response calls state_contains_service for every returned unknown hash after stripping only the trailing hash. The classic network connection forwards all block inventory entries in a FindBlocks inv as Response::BlockHashes, and received inventory messages are bounded by the much larger 50,000-item protocol cap, so a malicious peer can force tens of thousands of negative state lookups per response.

Analyzed one file, diff 7afa5bb...790a800.

@p0mvn
p0mvn merged commit ef61966 into security-ports Jul 14, 2026
2 checks passed
@p0mvn
p0mvn deleted the security-port/sync-discovery-validation branch July 14, 2026 04:29
@p0mvn p0mvn mentioned this pull request Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants