Skip to content

fix(inbound): retry duplicate block advertisers#152

Merged
p0mvn merged 2 commits into
security-portsfrom
security-port/inbound-block-fallbacks
Jul 14, 2026
Merged

fix(inbound): retry duplicate block advertisers#152
p0mvn merged 2 commits into
security-portsfrom
security-port/inbound-block-fallbacks

Conversation

@p0mvn

@p0mvn p0mvn commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Motivation

When multiple peers advertised the same gossiped block, the node previously kept only the first advertiser. A malicious first advertiser could race honest peers, then timeout or return malformed data, causing the gossip download to fail even though other peers had also advertised the block.

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

Solution

  • Record bounded fallback advertisers for active gossiped block downloads.
  • Retry eligible source failures with the next fallback source.
  • Avoid fallback retries for consensus-invalid matching blocks, including production-shaped verifier router errors.
  • Add regression tests for fallback retry after malformed data and no retry after consensus-invalid data.
  • Adapt the source patch's crate references to Zakura's renamed crates.

Tests

  • cargo fmt --all -- --check
  • cargo clippy --workspace --all-targets -- -D warnings
  • cargo test -p zakura components::inbound
  • 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, resolved Zakura crate-renaming compatibility, 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 reviewed 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 four issues worth reviewing.

Open the full results here.

FindingSeverityDetails
F-98772 🟡 Medium
Unbounded fallback retry budget

The fallback path lets a block hash be retried indefinitely because it tracks only the current active source and the currently queued fallback deque. After a non-consensus failure, poll_next pops one fallback and respawns the same hash without recording that the previous source has already been tried or decrementing a per-hash attempt budget. While the fallback source is active, the previously failed source is no longer active and no longer present in the fallback deque, so another duplicate advertisement from that source is accepted again. Two malicious peers can therefore alternate malformed responses and re-advertisements to keep the same hash in pending for as long as they keep the sequence going. With enough hashes, this keeps the inbound download queue at capacity and causes later honest block advertisements to be rejected as FullQueue.

F-98773 🟡 Medium
Local failures retry all fallbacks

Fallback retry classification is based only on verifier error types, so deterministic local validation failures are retried from every recorded alternate advertiser. The download task performs several pre-verifier checks after requesting and accepting a matching block body, including the coinbase-height presence check and the behind-tip / ahead-of-tip height bounds. These failures are returned as ordinary boxed string errors rather than as VerifyBlockError or RouterError::Block. A Sybil can advertise the same far-ahead or otherwise locally rejected hash through the active source and up to eight fallback sources, and each source can serve the same block body. The node downloads and rechecks the same deterministic failure for each fallback even though no peer can make that block acceptable at the current tip.

F-98774 🟡 Medium
Bad bodies suppress fallbacks

A malicious first advertiser can make a valid block hash look globally invalid by serving a block with the requested header hash but a corrupted body. The download task only checks block.hash() == hash before sending the full block to the verifier, and Block::hash() hashes the header rather than the transaction list. The transaction body is bound to the header later by the verifier’s Merkle-root check. If the attacker returns the real header with mutated transactions, the verifier rejects the body as a block verification error. The new retry policy treats every VerifyBlockError and every RouterError::Block as globally consensus-invalid, so it skips recorded honest fallback advertisers that could have supplied the valid body for the same hash.

F-98777 🟡 Medium
Router-wrapped consensus errors bypass retry and scoring logic

Inbound download error handling is inconsistent with the router’s wrapped verifier errors. The router surfaces consensus failures as RouterError::Block and RouterError::Checkpoint, but downstream logic still assumes failures will either be a bare VerifyBlockError or, in the retry classifier, only the Block wrapper. As a result, Inbound::poll_ready skips misbehavior accounting for router-wrapped failures instead of unwrapping them and applying their misbehavior_score(). For checkpoint-range blocks, the omission is worse because is_consensus_invalid_block_error() does not recognize RouterError::Checkpoint, so invalid checkpoint blocks are treated as retryable and can be fetched again from fallbacks. The implementation needs a shared unwrapping/classification path for both router variants so retry suppression and peer scoring use the same consensus-error view.

Analyzed one file, diff 7afa5bb...88dc8be.

@p0mvn
p0mvn merged commit e3922ad into security-ports Jul 14, 2026
2 checks passed
@p0mvn
p0mvn deleted the security-port/inbound-block-fallbacks 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