Skip to content

fix(zebrad): score inbound gossip block failures via RouterError downcast - #11228

Open
natalieesk wants to merge 1 commit into
mainfrom
inbound_gossip_misbehavior_score_10616
Open

fix(zebrad): score inbound gossip block failures via RouterError downcast#11228
natalieesk wants to merge 1 commit into
mainfrom
inbound_gossip_misbehavior_score_10616

Conversation

@natalieesk

Copy link
Copy Markdown
Contributor

Summary

Closes #10616.

  • zebrad/src/components/inbound.rs: the inbound gossip cleanup path in poll_ready now scores a completed block-download failure by trying a RouterError downcast first, falling back to VerifyBlockError. The extraction moves into a block_download_misbehavior_score helper.
  • CHANGELOG.md: Security entry.
  • Unit tests in zebrad/src/components/inbound/tests.rs.

Approach & Key Decisions

The inbound block verifier is the consensus router (inbound.rs: BoxService<…, RouterError>), so a verification failure arriving through gossip boxes a RouterError. The old code downcast that box only to VerifyBlockError, which never matches a RouterError, so scoring was silently skipped — a peer serving a score-100 invalid block was banned when the block came via sync but not via gossip. The sync path (sync/downloads.rs:569, sync.rs:1194) already works with RouterError and RouterError::misbehavior_score(); this change gives inbound the same behavior.

The scoring logic is pulled into block_download_misbehavior_score(err: BoxError) -> u32 so it can be unit-tested directly rather than through the full Inbound service. The VerifyBlockError branch is kept as a defensive fallback (not reached on the current router path). RouterError::misbehavior_score() delegates to the wrapped block error, so the scores match the sync path exactly. Only errors carrying Some(advertiser_addr) reach this code; timeouts and other non-attributable failures downcast to neither type and score 0, as before.

Testing & Verification

Three unit tests in inbound/tests.rs, using RouterError::from(VerifyBlockError::Subsidy(SubsidyError::NoCoinbase)) (score 100 — the same RouterError::Block { .. } shape the router emits):

  • router_error_yields_misbehavior_score — fails before the fix (returns 0), passes after (100). This is the regression.
  • verify_block_error_yields_misbehavior_score — fallback path returns 100.
  • unrelated_error_scores_zero — an unrelated boxed error returns 0.
cargo test -p zebrad --lib -- components::inbound::tests
test result: ok. 3 passed

cargo fmt and cargo clippy -p zebrad --lib --all-features are clean.

Risk & Impact

Behavior change confined to peer misbehavior scoring: invalid blocks gossiped by a peer now raise its score (and can lead to a ban), matching what already happens on the sync path. No consensus, state-format, RPC, or config change; no DB format bump. A misbehavior score is advisory input to ban logic, so the blast radius is limited to peer connection management.

Changelog

Added under ### Security in CHANGELOG.md: inbound gossiped invalid blocks now increase the sending peer's misbehavior score.

AI Disclosure

Claude (Claude Code) wrote the fix, the helper refactor, and the tests.

PR Checklist

  • CHANGELOG.md updated (Security)
  • DB format version unchanged (no state-format change)
  • Linked issue exists and was acknowledged by the team before work started

…cast

The inbound gossip cleanup path downcast completed block-download errors to
`VerifyBlockError` to extract a peer misbehavior score. But the inbound block
verifier is the consensus router, which returns `RouterError`, so the downcast
failed for normal router failures and scoring was silently skipped: a peer
serving a score-100 invalid block (e.g. `InvalidDifficulty`) is banned when it
arrives via sync, but went unpenalised through inbound gossip.

Extract the extraction into `block_download_misbehavior_score`, which tries a
`RouterError` downcast first and falls back to `VerifyBlockError`, mirroring the
sync download path. Unit-tests cover all three cases (RouterError,
VerifyBlockError, unrelated error).
@v12-auditor

v12-auditor Bot commented Aug 10, 2026

Copy link
Copy Markdown

Note

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

Open the full results here.

FindingSeverityDetails
F-206040 🟡 Medium
Heightless blocks bypass peer scoring

Inbound::poll_ready only scores failed downloads that carry Some(advertiser_addr), but the downloader discards the already-known serving-peer address when block.coinbase_height() returns None. A remote peer can advertise an arbitrary header hash and answer the resulting request with a deserializable zero-transaction block whose header matches that hash. Such a block is unconditionally consensus-invalid, and the normal verifier maps the equivalent missing-height failure to a 100-point score. Because the downloader instead returns (error, None), the changed helper is never called and the failure produces no score. The peer can repeat after each completion because the per-IP limit only bounds concurrent, not cumulative, attempts.

F-206041 🟡 Medium
Merkle-malleated blocks evade scoring

The Bitcoin-style transaction Merkle construction duplicates an unpaired trailing hash, so appending a copy of the final transaction to an odd-length valid block preserves its Merkle root, header, proof of work, and block hash. The network response handler therefore accepts the mutated body for the requested genuine hash, while consensus first passes the root comparison and then returns BlockError::DuplicateTransaction. BlockError::misbehavior_score assigns 100 to BadMerkleRoot but omits DuplicateTransaction, causing the latter to fall through to zero. RouterError and VerifyBlockError delegate that zero to the new helper, and Inbound::poll_ready consequently sends no report for the serving peer. Completion removes the in-flight hash/IP bookkeeping without caching the rejection, allowing the same peer and hash to repeat.

F-206042 🟡 Medium
Invalid script blocks evade scoring

For version-5-or-later transactions, the mined transaction ID excludes authorizing data, so an attacker can corrupt a transparent scriptSig in a genuine valid-PoW block without changing its transaction Merkle root, header, or block hash. Semantic verification reaches the CPU-bound script interpreter before the later contextual block-authorizing-data commitment check, and the script verifier boxes zebra_script::Error::ScriptInvalid. From<BoxError> for TransactionError does not downcast zebra_script::Error, converting the failure to InternalDowncastError instead of the existing Script variant. The score table assigns 100 to Script(_) but lets InternalDowncastError fall through to zero. Thus the new router downcast succeeds, but block_download_misbehavior_score still returns zero and the serving peer is not reported.

F-206043 🟡 Medium
Mutated checkpoint bodies evade scoring

For NU5-or-later blocks, transaction IDs and the transaction Merkle root do not bind signatures, proofs, or scripts, so a peer can mutate a genuine checkpoint-range block's authorizing data without changing its valid-PoW header or block hash. The checkpoint verifier accepts this body after checking height, proof of work, and the unchanged transaction Merkle root, then selects it in the checkpoint hash chain by the unchanged header hash. State later recomputes hashAuthDataRoot and rejects the mutated body, but that failure is returned as VerifyCheckpointError::CommitCheckpointVerified. The checkpoint score table does not classify that variant, so RouterError::Checkpoint reports zero to the new helper and Inbound::poll_ready sends no peer report. The checkpoint verifier then resets to the state tip, while the uncommitted block remains eligible to be downloaded and attempted again.

F-206044 🔵 Low
Reports can vanish silently

The new gossip scoring path reports a nonzero score with misbehavior_sender.try_send and immediately discards the result. If the bounded channel is full, or if the batch task has exited and the channel is closed, the only copy of that misbehavior report is lost. The loss is permanent because polling the completed download also removes the hash and per-IP in-flight bookkeeping, and the score is not stored anywhere else for retry. Neighboring downloader branches log or count their failures, but a dropped report has no log, metric, or fallback write to the address book. A remote-only saturation path was not confirmed, but the implementation is fail-open for a security-control delivery failure.

Analyzed one file, diff 05d129b...83012fa.

@natalieesk natalieesk added C-bug Category: This is a bug C-security Category: Security issues C-audit Category: Issues arising from audit findings A-network Area: Network protocol updates or fixes I-remote-trigger Remote nodes can make Zebra do something bad labels Aug 10, 2026
@mergify

mergify Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Merge Protections

🟠 1 of 1 protections blocking · waiting on 🕒 schedule

Protection Waiting on
🟠 ❄️ 6.3.0 release [Scheduled Freeze] 🕒 schedule

🟠 ❄️ 6.3.0 release [Scheduled Freeze]

Waiting for

  • current-datetime < 2026-08-10T14:25:11[America/Sao_Paulo]
This freeze has no end date and must be removed manually.

A freeze on the repository is scheduled for the following reason: 6.3.0 release

  • current-datetime < 2026-08-10T14:25:11[America/Sao_Paulo]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-network Area: Network protocol updates or fixes C-audit Category: Issues arising from audit findings C-bug Category: This is a bug C-security Category: Security issues I-remote-trigger Remote nodes can make Zebra do something bad

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inbound gossip block verification failures skip peer misbehavior scoring due to RouterError downcast mismatch

1 participant