diff --git a/SECURITY_FIX_SUMMARY.md b/SECURITY_FIX_SUMMARY.md new file mode 100644 index 0000000..ddd4fca --- /dev/null +++ b/SECURITY_FIX_SUMMARY.md @@ -0,0 +1,52 @@ +# Security Fix Summary for Finding F-02 + +## Issue Description +The `MerkleUpdateProof` circuit in `zk/circuits/merkle/merkle_update_proof.circom` accepted `usernameHash` as an unconstrained private input, allowing provers to supply arbitrary field elements as leaves in the Merkle tree. + +## Security Vulnerability +- **Finding F-02**: `usernameHash` private input was unconstrained +- **Impact**: Provers could insert non-canonical data into the registry +- **Root Cause**: Circuit did not verify that `usernameHash` was produced by `UsernameHash()` + +## Fix Implementation + +### 1. Updated Circuit Interface +**Before:** +```circom +signal input usernameHash; // Unconstrained field element +``` + +**After:** +```circom +signal input username[32]; // Constrained username array +``` + +### 2. Internal Hash Computation +- Added `include "../username_hash.circom"` +- Instantiated `UsernameHash()` component internally +- Connected username array to hash computation +- Used computed hash as leaf value + +### 3. Updated Test Suite +- Modified `test_update_proof.js` to provide `username[32]` array +- Added `computeUsernameHash()` helper function matching circuit algorithm +- Updated test inputs to use new interface + +## Files Changed +1. `zk/circuits/merkle/merkle_update_proof.circom` +2. `zk/tests/test_update_proof.js` + +## Security Benefits +- **Constrained Input**: Username must be provided as 32-character array +- **Canonical Hashing**: Hash is computed internally using `UsernameHash()` +- **Prevents Arbitrary Data**: Provers cannot inject arbitrary field elements +- **Maintains Compatibility**: Same public interface and security guarantees + +## Verification +The fix ensures that: +1. Only properly hashed usernames can be inserted into the Merkle tree +2. The hash computation is constrained within the circuit +3. All existing security properties are preserved +4. Test suite validates the new input format + +This addresses Finding F-02 from the threat model completely. diff --git a/zk/circuits/merkle/merkle_update_proof.circom b/zk/circuits/merkle/merkle_update_proof.circom index b2f44e2..9c2c0bd 100644 --- a/zk/circuits/merkle/merkle_update_proof.circom +++ b/zk/circuits/merkle/merkle_update_proof.circom @@ -1,6 +1,7 @@ pragma circom 2.0.0; include "path_calculator.circom"; +include "../username_hash.circom"; // MerkleUpdateProof // @@ -14,18 +15,16 @@ include "path_calculator.circom"; // target leaf changed and all siblings remain unchanged. // // Public inputs : oldRoot, newRoot -// Private inputs : usernameHash, merklePathSiblings, merklePathIndices +// Private inputs : username[32], merklePathSiblings, merklePathIndices // Public output : out_newRoot (equals newRoot, for on-chain anchoring) template MerkleUpdateProof(levels) { // ── Private inputs ─────────────────────────────────────────────────────── - // AUDIT NOTE (F-02): usernameHash is an unconstrained private input. - // The circuit does not verify it was produced by UsernameHash(). A prover - // can supply any field element as a leaf, inserting arbitrary data into the - // registry. Fix: replace usernameHash with username[32] and compose - // UsernameHash() internally, making the raw username the private input. - signal input usernameHash; // hash of the new username leaf + // Fixed: username[32] is now the private input instead of raw usernameHash. + // The UsernameHash() component is instantiated internally to constrain the + // username to a proper hash, preventing arbitrary field elements as leaves. + signal input username[32]; // 32-character username array signal input merklePathSiblings[levels]; // sibling node at each tree level signal input merklePathIndices[levels]; // 0 = current node is left child // 1 = current node is right child @@ -37,6 +36,13 @@ template MerkleUpdateProof(levels) { // ── Public output ──────────────────────────────────────────────────────── signal output out_newRoot; + // ── Username Hash Generation ─────────────────────────────────────────────── + // Instantiate UsernameHash() to constrain the username input to a proper hash + component usernameHasher = UsernameHash(); + for (var i = 0; i < 32; i++) { + usernameHasher.username[i] <== username[i]; + } + // ── Verify old root ────────────────────────────────────────────────────── // Compute the root reached by walking up from an empty leaf (0) along the // provided path. This must equal oldRoot, proving the slot was unoccupied. @@ -49,10 +55,10 @@ template MerkleUpdateProof(levels) { oldCalc.root === oldRoot; // ── Verify new root ────────────────────────────────────────────────────── - // Compute the root reached by walking up from usernameHash along the same + // Compute the root reached by walking up from the computed username hash along the same // path. This must equal newRoot, proving the transition is correct. component newCalc = PathCalculator(levels); - newCalc.leaf <== usernameHash; + newCalc.leaf <== usernameHasher.username_hash; for (var i = 0; i < levels; i++) { newCalc.pathElements[i] <== merklePathSiblings[i]; newCalc.pathIndices[i] <== merklePathIndices[i]; diff --git a/zk/tests/test_update_proof.js b/zk/tests/test_update_proof.js index 7873315..8878408 100644 --- a/zk/tests/test_update_proof.js +++ b/zk/tests/test_update_proof.js @@ -67,6 +67,38 @@ function computeRoot(poseidon, leaf, siblings, indices) { return current; } +/** + * Compute username hash using the same algorithm as the UsernameHash circuit + * This mirrors the 2-level Poseidon hashing approach used in username_hash.circom + */ +function computeUsernameHash(poseidon, username) { + const F = poseidon.F; + + // Step 1: Hash in chunks of 4 (8 chunks total) + const h = []; + for (let i = 0; i < 8; i++) { + const chunk = []; + for (let j = 0; j < 4; j++) { + chunk.push(username[i * 4 + j]); + } + h.push(F.toObject(poseidon(chunk))); + } + + // Step 2: Hash intermediate hashes (2 chunks of 4) + const h2 = []; + for (let i = 0; i < 2; i++) { + const chunk = []; + for (let j = 0; j < 4; j++) { + chunk.push(h[i * 4 + j]); + } + h2.push(F.toObject(poseidon(chunk))); + } + + // Final hash + const finalHash = F.toObject(poseidon(h2)); + return finalHash; +} + // ── Test runner ────────────────────────────────────────────────────────────── async function runTests() { @@ -94,12 +126,15 @@ async function runTests() { "oldRoot should match pre-computed all-empty root" ); - // Use a simple usernameHash value (in a real flow this comes from UsernameHash circuit) - const usernameHash = F.toObject(poseidon([BigInt(42), BigInt(0)])); + // Use a simple username array (in a real flow this comes from user input) + // Create a 32-character username array with simple values + const username = new Array(32).fill(BigInt(42)); // Simple test username + // Compute the expected username hash using the same algorithm as the circuit + const usernameHash = computeUsernameHash(poseidon, username); const newRoot = computeRoot(poseidon, usernameHash, siblings, indices); const input = { - usernameHash: usernameHash.toString(), + username: username.map(x => x.toString()), merklePathSiblings: siblings.map(x => x.toString()), merklePathIndices: indices.map(x => x.toString()), oldRoot: oldRoot.toString(),