Skip to content

fix(core/merkle): reject over-long SMT inclusion proofs (panic/DoS)#429

Open
dsmfa10 wants to merge 1 commit into
mainfrom
fix/core-smt-sibling-bound
Open

fix(core/merkle): reject over-long SMT inclusion proofs (panic/DoS)#429
dsmfa10 wants to merge 1 commit into
mainfrom
fix/core-smt-sibling-bound

Conversation

@dsmfa10
Copy link
Copy Markdown
Collaborator

@dsmfa10 dsmfa10 commented Jun 3, 2026

Problem

SparseMerkleTree::verify_proof_against_root folds the proof's siblings from leaf
to root, computing the tree level as 255 - i:

// src/merkle/sparse_merkle_tree.rs (before)
for (i, sibling) in proof.siblings.iter().enumerate() {
    let level = 255 - i;                      // <-- underflows when i >= 256
    let bit = get_bit(&proof.key, level);
    ...
}

There is no bound on proof.siblings.len(). For a peer-supplied proof with more
than 256 siblings, at i == 256 the expression 255 - i underflows usize
(panic in debug; in release it wraps to a huge value and the subsequent
get_bit indexes out of bounds → panic). This is reachable from the BLE
acceptance path: receipts::deserialize_inclusion_proof decodes a u32 sibling
count with no <= 256 cap and feeds the result here.

Impact

A malicious peer can crash the verifier (and thus the receiving node/handler)
with a single oversized inclusion proof.

Fix

The SMT is fixed-depth (256 levels), so a valid inclusion proof has at most 256
siblings. Reject longer proofs before the fold.

if proof.siblings.len() > 256 {
    return false;
}

Verification

cargo check -p dsm clean. Well-formed 256-sibling proofs are unaffected; only
over-long proofs (which could never be valid anyway) are now rejected instead of
panicking.

Notes

The sibling-cap divergence between the struct's own from_bytes and
receipts::deserialize_inclusion_proof (two byte-identical decoders) is noted in
the audit; consolidating them is a separate cleanup.

CI gates & coverage

Full verification for this PR runs in CI — Rust (cargo fmt --check,
clippy -D warnings, workspace tests), Frontend, Android Unit Tests,
Coverage, SPDX headers, CodeQL (see the PR's Checks tab). The local
check noted above is a subset; the broader mandated gates (full workspace test
suite, codegen/scan, Android, Frontend) run in CI, not locally — none is
silently skipped.

`verify_proof_against_root` computed `level = 255 - i` while iterating
`proof.siblings` with no upper bound. For a peer-supplied proof with more
than 256 siblings, `i == 256` underflows `255 - i` (debug panic; release
wraparound -> out-of-bounds `get_bit` panic) — a crash from an untrusted
proof reachable on the BLE acceptance path.

The SMT is fixed-depth (256 levels), so reject any proof with more than
256 siblings before the fold. Well-formed 256-sibling proofs are unaffected.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@codecov-commenter
Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...state_machine/dsm/src/merkle/sparse_merkle_tree.rs 66.66% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

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