Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions SECURITY_FIX_SUMMARY.md
Original file line number Diff line number Diff line change
@@ -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.
24 changes: 15 additions & 9 deletions zk/circuits/merkle/merkle_update_proof.circom
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
pragma circom 2.0.0;

include "path_calculator.circom";
include "../username_hash.circom";

// MerkleUpdateProof
//
Expand All @@ -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
Expand All @@ -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.
Expand All @@ -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];
Expand Down
41 changes: 38 additions & 3 deletions zk/tests/test_update_proof.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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(),
Expand Down
Loading