Description
merkle::hash_leaf and merkle::hash_pair both compute a plain, unprefixed SHA-256 digest:
/// Hash two child nodes together: SHA-256(left || right).
pub fn hash_pair(env: &Env, left: &Bytes, right: &Bytes) -> Bytes {
let mut data = Bytes::new(env);
data.append(left);
data.append(right);
env.crypto().sha256(&data).into()
}
/// Hash a leaf value: SHA-256(leaf_data).
pub fn hash_leaf(env: &Env, data: &Bytes) -> Bytes {
env.crypto().sha256(data).into()
}
Both functions share the exact same hash domain: hash_leaf(x) == sha256(x) and hash_pair(a, b) == sha256(a || b). Since hash_leaf accepts arbitrary-length Bytes, nothing stops x from being the 64-byte concatenation of two other nodes' 32-byte hashes. If x = a || b for some pair of hashes a, b that are themselves siblings at some internal level of a previously-committed tree, then hash_leaf(x) == sha256(a || b) == hash_pair(a, b) — the same value the tree already uses as that internal node's hash. This means a party who knows any two adjacent node hashes anywhere in the tree can construct a fake MerkleProof whose leaf field is a || b and whose siblings are simply the real siblings from that internal node's level up to the root, and verify_proof will accept it as a legitimate leaf inclusion proof — even though a || b was never one of the originally committed evidence leaves. This is the standard "second-preimage" weakness that comes from not distinguishing leaf hashes from internal-node hashes in a Merkle tree; it's why RFC 6962 (Certificate Transparency) mandates a 0x00 prefix for leaf hashes and 0x01 for internal-node hashes specifically to make this collision structurally impossible.
commit_evidence_root (this file, lines 11-50) trusts whatever root the contributor commits, and verify_proof (lines 53-72) is the sole on-chain gate that decides whether a piece of evidence was really part of the committed set — so a forged proof accepted here would let a party claim inclusion of evidence data that was never actually part of the leaf set the contributor originally committed to.
Technical Requirements
Files to update
contracts/contracts/stellar-grants/src/merkle.rs (hash_leaf, lines 83-85; hash_pair, lines 75-80)
contracts/contracts/stellar-grants/src/merkle.rs test module and contracts/contracts/stellar-grants/tests/merkle_test.rs — both build known-vector roots by calling hash_leaf/hash_pair directly, so they'll need to be re-derived against the new prefixed hashing (no hardcoded/independent root values to break).
Fix direction
Prefix each hash input with a fixed domain tag before hashing, e.g.:
pub fn hash_leaf(env: &Env, data: &Bytes) -> Bytes {
let mut input = Bytes::from_array(env, &[0x00u8]);
input.append(data);
env.crypto().sha256(&input).into()
}
pub fn hash_pair(env: &Env, left: &Bytes, right: &Bytes) -> Bytes {
let mut input = Bytes::from_array(env, &[0x01u8]);
input.append(left);
input.append(right);
env.crypto().sha256(&input).into()
}
Acceptance Criteria
hash_leaf and hash_pair use distinct, non-overlapping hash domains such that no leaf input can be crafted to collide with any internal-node hash.
- A test demonstrates that a proof built from two real sibling hashes concatenated as a fake "leaf" (the attack described above) no longer verifies against the real root.
- Existing tests (
test_hash_pair_is_order_sensitive, test_tampered_leaf_fails_verification, test_four_leaf_known_vector_root_and_proof, test_tampered_leaf_rejected) are updated for the new hash values and still pass.
cargo test passes.
Estimated Effort
Beginner: 5 hours
Intermediate: 2.5 hours
Expert: 1.5 hours
How to work this issue
- Read
contracts/ContributionGuide.md for the contribution workflow.
- Comment on the issue to claim it before starting.
- Branch:
fix/issue-121-merkle-hash-domain-separation.
- Run
cargo fmt, cargo clippy -- -D warnings, cargo test before opening your PR.
- Use a Conventional Commit message, e.g.
security: add domain separation to merkle leaf/internal-node hashing.
Before you start
If you find this project interesting, please consider starring the repository on GitHub. It helps the project gain visibility and supports the Drips Wave program that rewards contributors for merged fixes like this one.
Description
merkle::hash_leafandmerkle::hash_pairboth compute a plain, unprefixed SHA-256 digest:Both functions share the exact same hash domain:
hash_leaf(x) == sha256(x)andhash_pair(a, b) == sha256(a || b). Sincehash_leafaccepts arbitrary-lengthBytes, nothing stopsxfrom being the 64-byte concatenation of two other nodes' 32-byte hashes. Ifx = a || bfor some pair of hashesa,bthat are themselves siblings at some internal level of a previously-committed tree, thenhash_leaf(x) == sha256(a || b) == hash_pair(a, b)— the same value the tree already uses as that internal node's hash. This means a party who knows any two adjacent node hashes anywhere in the tree can construct a fakeMerkleProofwhoseleaffield isa || band whosesiblingsare simply the real siblings from that internal node's level up to the root, andverify_proofwill accept it as a legitimate leaf inclusion proof — even thougha || bwas never one of the originally committed evidence leaves. This is the standard "second-preimage" weakness that comes from not distinguishing leaf hashes from internal-node hashes in a Merkle tree; it's why RFC 6962 (Certificate Transparency) mandates a0x00prefix for leaf hashes and0x01for internal-node hashes specifically to make this collision structurally impossible.commit_evidence_root(this file, lines 11-50) trusts whatever root the contributor commits, andverify_proof(lines 53-72) is the sole on-chain gate that decides whether a piece of evidence was really part of the committed set — so a forged proof accepted here would let a party claim inclusion of evidence data that was never actually part of the leaf set the contributor originally committed to.Technical Requirements
Files to update
contracts/contracts/stellar-grants/src/merkle.rs(hash_leaf, lines 83-85;hash_pair, lines 75-80)contracts/contracts/stellar-grants/src/merkle.rstest module andcontracts/contracts/stellar-grants/tests/merkle_test.rs— both build known-vector roots by callinghash_leaf/hash_pairdirectly, so they'll need to be re-derived against the new prefixed hashing (no hardcoded/independent root values to break).Fix direction
Prefix each hash input with a fixed domain tag before hashing, e.g.:
Acceptance Criteria
hash_leafandhash_pairuse distinct, non-overlapping hash domains such that no leaf input can be crafted to collide with any internal-node hash.test_hash_pair_is_order_sensitive,test_tampered_leaf_fails_verification,test_four_leaf_known_vector_root_and_proof,test_tampered_leaf_rejected) are updated for the new hash values and still pass.cargo testpasses.Estimated Effort
Beginner: 5 hours
Intermediate: 2.5 hours
Expert: 1.5 hours
How to work this issue
contracts/ContributionGuide.mdfor the contribution workflow.fix/issue-121-merkle-hash-domain-separation.cargo fmt,cargo clippy -- -D warnings,cargo testbefore opening your PR.security: add domain separation to merkle leaf/internal-node hashing.Before you start
If you find this project interesting, please consider starring the repository on GitHub. It helps the project gain visibility and supports the Drips Wave program that rewards contributors for merged fixes like this one.