fix(btc-verification): bind inclusion proof to the block's Merkle tree - #175
Merged
Conversation
TxidInclusionProof::verify reconstructs the Merkle root from the supplied
siblings without binding the proof to the tree structure. Nothing checks the
number of siblings against the expected tree depth, nor that the position is a
valid leaf index. This lets an attacker forge inclusion:
- a zero-length proof passes off any single hash (e.g. an internal Merkle
node, which under Bitcoin's 64-byte tx ambiguity can also be a valid txid)
as a whole-block root; and
- an out-of-range position verifies against the real Merkle root, since only
the low siblings.len() bits of the position feed left/right ordering.
Add a test that exercises both to lock in the current (vulnerable) behavior
before the fix.
TxidInclusionProof::verify folded the supplied siblings into a root without
checking them against the tree the root commits to, so it proved a weaker
statement than intended ("some fold reaches the root") rather than "the leaf
sits at index i of an N-leaf tree". A zero-length proof passed off any single
hash as a whole-block root, and out-of-range positions verified because only
the low siblings.len() bits feed left/right ordering.
Require the caller to supply the block's transaction count and reject any proof
whose position is not a valid leaf index or whose sibling count differs from the
tree depth ceil(log2(tx_count)). The depth check is Bitcoin Core's standard
mitigation against the 64-byte node/transaction ambiguity, where an internal
node presented as a leaf yields a proof shorter than the true depth.
Not known to be exploitable in the current wiring: the sole caller checks the
coinbase against a PoW-committed root and gates on is_coinbase(), so a forgery
would need a full preimage on the root or a 64-byte internal node that is also a
structurally valid coinbase. This closes the gap so the verifier is sound on its
own rather than relying on those external invariants.
prajwolrg
force-pushed
the
fix/inclusion-proof-depth-binding
branch
from
July 2, 2026 06:46
a3d42dc to
6571f31
Compare
Codecov Report✅ All modified and coverable lines are covered by tests.
... and 3 files with indirect coverage changes 🚀 New features to boost your workflow:
|
|
Commit: 0ad621f
|
Collaborator
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
MdTeach
approved these changes
Jul 5, 2026
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
2 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.
Description
TxidInclusionProof::verifyreconstructed the Merkle root by folding the supplied siblings into the leaf txid, but never checked those siblings against the tree the root actually commits to. It therefore proved a weaker statement than intended — "some fold reaches the root" rather than "the leaf sits at index i of an N-leaf tree". Two consequences, both covered by the added test:siblings.len()bits of the position feed left/right ordering.The fix requires the caller to pass the block's transaction count and rejects any proof whose position is not a valid leaf index (
< tx_count) or whose sibling count differs from the tree depthceil(log2(tx_count)). The depth check is Bitcoin Core's standard mitigation against the 64-byte node/transaction ambiguity, where an internal node presented as a leaf yields a proof shorter than the true tree depth.Notes to Reviewers
This is soundness hardening of the proof verifier, not a patch for an actively exploitable bug. The sole production caller (
check_block_integrity) verifies the coinbase against a PoW-committedheader.merkle_rootand gates onis_coinbase(), and the guest always has the full block. A working forgery would therefore need either a full 256-bit preimage on the committed root, or a 64-byte string that is simultaneously a real block's internal Merkle node and a structurally valid coinbase — neither is feasible. The change makesverifysound on its own rather than relying on those external invariants, which matters becauseverify/compute_rootarepuband reusable elsewhere.Separately: the coinbase inclusion proof barely optimizes anything, since the segwit path already walks all of
txdatato compute the witness root. Recomputing the txid Merkle root directly (as the non-segwit path does) would bind the coinbase fully and make the proof — and this whole bug class — unnecessary. Worth a follow-up ticket; out of scope here.Type of Change
Checklist