Skip to content

fix: add domain separation and depth bound to Merkle verification - #830

Open
Awesome254 wants to merge 5 commits into
zintarh:mainfrom
Awesome254:fix/reopen-650
Open

fix: add domain separation and depth bound to Merkle verification#830
Awesome254 wants to merge 5 commits into
zintarh:mainfrom
Awesome254:fix/reopen-650

Conversation

@Awesome254

Copy link
Copy Markdown

Overview

This PR hardens merkle::verify_merkle_proof against second-preimage attacks and excessive proof lengths. Leaf hashes are now prefixed with 0x00, internal node hashes with 0x01, and proof iteration is capped at MAX_PROOF_DEPTH = 32. The Rust merkle implementation and scripts/merkle.ts are aligned, and the encoding/depth behavior is documented with cross-implementation test vectors.

Related Issue

Changes

🔒 Merkle Verification Hardening

  • [MODIFY] src/constants.rs

    • Add MERKLE_LEAF_PREFIX = 0x00, MERKLE_INTERNAL_PREFIX = 0x01, and MAX_PROOF_DEPTH = 32.
  • [MODIFY] src/errors.rs

    • Add a dedicated MerkleProofTooLong error for proofs longer than MAX_PROOF_DEPTH.
  • [MODIFY] src/merkle.rs

    • Prefix leaf and internal-node inputs before hashing to prevent leaf/node confusion.
    • Enforce the proof-length bound inside verify_merkle_proof; reject longer proofs with MerkleProofTooLong.
    • Preserve the empty-proof decision: verify_merkle_proof returns leaf == root for a single-member tree.
  • [MODIFY] src/merkle_test.rs

    • Add regression tests for leaf/node domain separation, MAX_PROOF_DEPTH rejection, empty-proof behavior, and cross-implementation vectors.
  • [MODIFY] scripts/merkle.ts

    • Match the Rust builder by hashing leaves as 0x00 ‖ leaf_data and internal nodes as 0x01 ‖ left ‖ right.

📚 Documentation

  • [MODIFY] docs/whitelist-merkle.md
    • Document the 0x00/0x01 encodings and the 32-level depth bound.
    • Explicitly describe the empty-proof case as a valid, tested single-member tree.

🧪 Cross-Implementation Test Vectors

  • Rust and TypeScript builders now agree on the same domain-separated Merkle roots for equivalent leaf sets.
  • New test vectors cover one-member and multi-member trees to verify compatibility.

Verification Results

cargo test --lib merkle_test
✅ 12/12 passed

node scripts/merkle.test.ts
✅ 5/5 passed

Live acceptance check:
✅ Domain-separated leaf/node hashes match between Rust and scripts/merkle.ts
✅ Proof lengths > 32 rejected with dedicated MerkleProofTooLong error
✅ Empty-proof single-member tree documented and tested
✅ docs/whitelist-merkle.md updated
Acceptance Criteria Status
Leaf and node hashing use distinct domain prefixes, matching scripts/merkle.ts 0x00 leaf / 0x01 internal prefixes in Rust and TypeScript; test vectors match
Proofs longer than MAX_PROOF_DEPTH are rejected with a dedicated error MerkleProofTooLong returned for depth > 32
docs/whitelist-merkle.md documents the encoding and the depth bound ✅ Encoding and MAX_PROOF_DEPTH documented
Cross-implementation test vectors verify the Rust and TypeScript builders agree ✅ Rust/TS builder vectors match for single and multi-member trees

Closes #650

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.

[Security] Merkle verification lacks leaf/node domain separation and a proof-length bound

1 participant