From 5173c395efa5000640a155c6f629cd1e5e970b0b Mon Sep 17 00:00:00 2001 From: Prajwol Gyawali Date: Thu, 2 Jul 2026 12:22:49 +0545 Subject: [PATCH 1/3] test(btc-verification): reproduce inclusion-proof forgery 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. --- .../btc-verification/src/inclusion_proof.rs | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/crates/btc-verification/src/inclusion_proof.rs b/crates/btc-verification/src/inclusion_proof.rs index d36e5136..88a5a091 100644 --- a/crates/btc-verification/src/inclusion_proof.rs +++ b/crates/btc-verification/src/inclusion_proof.rs @@ -151,4 +151,47 @@ mod tests { assert!(proof.verify(tx, merkle_root)); } } + + /// Reproduces the inclusion-proof forgery: `verify` never binds the proof to the tree + /// structure, so it accepts proofs of the wrong length and out-of-range positions. + #[test] + fn test_forged_inclusion_proof_is_accepted() { + let block = BtcMainnetSegment::load_full_block(); + let merkle_root: Buf32 = block.header.merkle_root.to_byte_array().into(); + let txs = &block.txdata; + assert!( + txs.len() > 1, + "need a multi-transaction block to demonstrate the forgery" + ); + + let coinbase = &txs[0]; + + // Forgery 1: a zero-length proof that claims the coinbase's own txid is the Merkle root. + // With no depth binding, `compute_root` just returns the leaf, so any single hash can be + // passed off as a whole-block root. This is the primitive behind the 64-byte node/tx + // second-preimage attack: an internal Merkle node presented as a leaf produces a proof + // shorter than the true tree depth, which the verifier cannot currently detect. + let empty_proof = TxidInclusionProof::new(0, vec![]); + let coinbase_txid = compute_txid(coinbase).to_buf32(); + assert!( + empty_proof.verify(coinbase, coinbase_txid), + "BUG: zero-length proof was rejected — the depth-binding fix may already be present" + ); + + // Forgery 2: an out-of-range position verifies against the *real* Merkle root. Only the + // low `siblings.len()` bits of the position feed left/right ordering, so `position` and + // `position + 2^depth` compute the same root — yet the latter is not a valid leaf index. + let valid = TxidInclusionProof::generate(txs, 0); + let depth = valid.siblings().len() as u32; + let bogus_position = 1u32 << depth; + assert!( + bogus_position >= txs.len() as u32, + "bogus position should be out of range" + ); + let forged_position = TxidInclusionProof::new(bogus_position, valid.siblings().to_vec()); + assert!( + forged_position.verify(coinbase, merkle_root), + "BUG: out-of-range position was rejected — the position check may already be present" + ); + } } From 6571f3129f555fb4ed391923477366019c7d6332 Mon Sep 17 00:00:00 2001 From: Prajwol Gyawali Date: Thu, 2 Jul 2026 12:28:25 +0545 Subject: [PATCH 2/3] fix(btc-verification): bind inclusion proof to the block's Merkle tree 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. --- .../btc-verification/src/body_verification.rs | 9 +- .../btc-verification/src/inclusion_proof.rs | 94 +++++++++++++------ 2 files changed, 72 insertions(+), 31 deletions(-) diff --git a/crates/btc-verification/src/body_verification.rs b/crates/btc-verification/src/body_verification.rs index 51aab9a4..0354f14f 100644 --- a/crates/btc-verification/src/body_verification.rs +++ b/crates/btc-verification/src/body_verification.rs @@ -82,8 +82,13 @@ pub fn check_block_integrity( return Err(L1BodyError::WitnessCommitmentMismatch); } - // Check the coinbase inclusion proof. - if !proof.verify(coinbase, header.merkle_root.to_byte_array().into()) { + // Check the coinbase inclusion proof. The transaction count comes from the block body, + // binding the proof to the block's actual Merkle tree. + if !proof.verify( + coinbase, + header.merkle_root.to_byte_array().into(), + txdata.len(), + ) { return Err(L1BodyError::InvalidInclusionProof); } diff --git a/crates/btc-verification/src/inclusion_proof.rs b/crates/btc-verification/src/inclusion_proof.rs index 88a5a091..0946b351 100644 --- a/crates/btc-verification/src/inclusion_proof.rs +++ b/crates/btc-verification/src/inclusion_proof.rs @@ -127,11 +127,40 @@ impl TxidInclusionProof { } /// Verifies the inclusion proof of the given `transaction` against the provided Merkle `root`. - pub fn verify(&self, transaction: &Transaction, root: Buf32) -> bool { + /// + /// `tx_count` is the number of transactions in the block the `root` commits to. It binds the + /// proof to the block's actual Merkle tree and must be sourced independently of the proof + /// (e.g. from the block body), never from the proof itself. + /// + /// The proof is rejected unless: + /// + /// - `tx_count` is non-zero; + /// - [`position`](Self::position) is a valid leaf index (`< tx_count`); and + /// - the number of siblings equals the tree depth `ceil(log2(tx_count))`. + /// + /// The depth check is Bitcoin Core's standard mitigation against the 64-byte node/transaction + /// ambiguity: an internal Merkle node presented as a leaf yields a proof shorter than the true + /// tree depth, so pinning the sibling count to the depth makes such forgeries unverifiable. + pub fn verify(&self, transaction: &Transaction, root: Buf32, tx_count: usize) -> bool { + if tx_count == 0 || self.position as usize >= tx_count { + return false; + } + if self.siblings.len() != merkle_tree_depth(tx_count) { + return false; + } self.compute_root(transaction) == root } } +/// Returns the depth of a Bitcoin Merkle tree with `tx_count` leaves, i.e. the number of sibling +/// hashes on the path from any leaf to the root: `ceil(log2(tx_count))`, and `0` for a single leaf. +fn merkle_tree_depth(tx_count: usize) -> usize { + match tx_count { + 0 | 1 => 0, + n => (usize::BITS - (n - 1).leading_zeros()) as usize, + } +} + #[cfg(test)] mod tests { use bitcoin::hashes::Hash; @@ -148,50 +177,57 @@ mod tests { for (idx, tx) in txs.iter().enumerate() { let proof = TxidInclusionProof::generate(txs, idx as u32); - assert!(proof.verify(tx, merkle_root)); + assert!(proof.verify(tx, merkle_root, txs.len())); } } - /// Reproduces the inclusion-proof forgery: `verify` never binds the proof to the tree - /// structure, so it accepts proofs of the wrong length and out-of-range positions. + /// Guards against the inclusion-proof forgery: binding the proof to the block's tree depth and + /// leaf count makes wrong-length proofs and out-of-range positions unverifiable. #[test] - fn test_forged_inclusion_proof_is_accepted() { + fn test_forged_inclusion_proof_is_rejected() { let block = BtcMainnetSegment::load_full_block(); let merkle_root: Buf32 = block.header.merkle_root.to_byte_array().into(); let txs = &block.txdata; - assert!( - txs.len() > 1, - "need a multi-transaction block to demonstrate the forgery" - ); + let tx_count = txs.len(); + assert!(tx_count > 1, "need a multi-transaction block"); let coinbase = &txs[0]; // Forgery 1: a zero-length proof that claims the coinbase's own txid is the Merkle root. - // With no depth binding, `compute_root` just returns the leaf, so any single hash can be - // passed off as a whole-block root. This is the primitive behind the 64-byte node/tx - // second-preimage attack: an internal Merkle node presented as a leaf produces a proof - // shorter than the true tree depth, which the verifier cannot currently detect. + // Rejected because the sibling count no longer matches the tree depth. This is the + // primitive behind the 64-byte node/tx second-preimage attack: an internal Merkle node + // presented as a leaf produces a proof shorter than the true tree depth. let empty_proof = TxidInclusionProof::new(0, vec![]); let coinbase_txid = compute_txid(coinbase).to_buf32(); - assert!( - empty_proof.verify(coinbase, coinbase_txid), - "BUG: zero-length proof was rejected — the depth-binding fix may already be present" - ); + assert!(!empty_proof.verify(coinbase, coinbase_txid, tx_count)); - // Forgery 2: an out-of-range position verifies against the *real* Merkle root. Only the - // low `siblings.len()` bits of the position feed left/right ordering, so `position` and - // `position + 2^depth` compute the same root — yet the latter is not a valid leaf index. + // Forgery 2: an out-of-range position that verifies against the real Merkle root because + // only the low `siblings.len()` bits feed left/right ordering. Rejected by the leaf-index + // bound. let valid = TxidInclusionProof::generate(txs, 0); - let depth = valid.siblings().len() as u32; - let bogus_position = 1u32 << depth; - assert!( - bogus_position >= txs.len() as u32, - "bogus position should be out of range" - ); - let forged_position = TxidInclusionProof::new(bogus_position, valid.siblings().to_vec()); + let depth = valid.siblings().len(); + let bogus_position = 1usize << depth; assert!( - forged_position.verify(coinbase, merkle_root), - "BUG: out-of-range position was rejected — the position check may already be present" + bogus_position >= tx_count, + "position should be out of range" ); + let forged_position = + TxidInclusionProof::new(bogus_position as u32, valid.siblings().to_vec()); + assert!(!forged_position.verify(coinbase, merkle_root, tx_count)); + + // The genuine proof still verifies. + assert!(valid.verify(coinbase, merkle_root, tx_count)); + } + + #[test] + fn test_merkle_tree_depth() { + // ceil(log2(n)); 0 for a single leaf. + assert_eq!(merkle_tree_depth(1), 0); + assert_eq!(merkle_tree_depth(2), 1); + assert_eq!(merkle_tree_depth(3), 2); + assert_eq!(merkle_tree_depth(4), 2); + assert_eq!(merkle_tree_depth(5), 3); + assert_eq!(merkle_tree_depth(8), 3); + assert_eq!(merkle_tree_depth(9), 4); } } From 39dc4832b359b9dd243b5cf8ed6c4cbd8610f96f Mon Sep 17 00:00:00 2001 From: vladb-ai Date: Thu, 2 Jul 2026 14:31:29 +0000 Subject: [PATCH 3/3] chore: trigger AI security review