SNARKeling Treasure Hunt

First Flight #59
Beginner FriendlyGameFiFoundry
100 EXP
View results
Submission Details
Severity: high
Valid

### [H-2] Duplicate hash in `ALLOWED_TREASURE_HASHES` allows a single proof to claim two rewards, permanently locking one treasure slot

Duplicate hash in ALLOWED_TREASURE_HASHES allows a single proof to claim two rewards, permanently locking one treasure slot

Description

  • The circuit defines 10 allowed treasure hashes baked into the ALLOWED_TREASURE_HASHES constant. The is_allowed() helper checks membership only, not uniqueness, meaning only 9 unique treasures exist.

  • Indices 8 and 9 are identical, so the holder of the proof for the hash at index 8 can submit two valid claims — one per unique treasureHash entry in claimed mapping — collecting 20 ETH instead of 10 ETH. The 10th unique treasure slot is unreachable by any other participant, meaning one reward is either stolen or permanently locked depending on execution order. This is compounded by [H-1] — with replay protection already broken, the same proof can be submitted MAX_TREASURES times draining the full contract before the duplicate even matters.

global ALLOWED_TREASURE_HASHES: [Field; 10] = [
...
-961435057317293580094826482786572873533235701183329831124091847635547871092, // index 8
-961435057317293580094826482786572873533235701183329831124091847635547871092 // index 9 — duplicate
];
fn is_allowed(hash: Field) -> bool {
let mut ok = false;
for i in 0..10 {
if hash == ALLOWED_TREASURE_HASHES[i] {
ok = true; // @> membership check only, no uniqueness enforced
}
}
ok // @> returns true for both index 8 and 9 with the same input
}

Risk

Likelihood:

  • Any holder of the proof corresponding to the duplicate hash at index 8 will be able to submit two valid claims, as both index 8 and index 9 pass the is_allowed() check with the same input.

  • The duplicate exists statically in the compiled circuit constant, making this exploitable on every deployment without any special precondition.

Impact:

  • One proof holder can claim 20 ETH instead of 10 ETH, doubling their reward at the protocol's expense.

  • One treasure slot is permanently unclaimable by any legitimate unique participant, contradicting the circuit comment: "This allows the prover to demonstrate knowledge of a valid treasure without revealing which one it is" — only 9 distinct secrets exist, not 10.

Proof of Concept

function test_duplicateHashClaimsTwice() public {
(bytes memory proof, bytes32 treasureHash, address payable recipient) = _loadFixture();
// Confirm the fixture proof corresponds to the duplicate hash (index 8 and 9)
// by checking it passes verification twice with the same treasureHash
address caller = makeAddr("caller");
uint256 recipientBalanceBefore = recipient.balance;
// First claim — valid, expected
vm.prank(caller);
hunt.claim(proof, treasureHash, recipient);
assertEq(recipient.balance, recipientBalanceBefore + 10 ether);
// Second claim — same proof, same hash, should revert but doesn't
// `claimed[treasureHash]` is set, but `claimed[_treasureHash]` check never fires (H-1)
// Even with H-1 fixed, duplicate in circuit means two valid proofs exist for same secret
vm.prank(caller);
hunt.claim(proof, treasureHash, recipient);
assertEq(recipient.balance, recipientBalanceBefore + 20 ether);
assertEq(hunt.claimsCount(), 2);
}

Recommended Mitigation

Replace the duplicate hash at index 9 with a unique valid pedersen hash corresponding to a distinct treasure secret:

global ALLOWED_TREASURE_HASHES: [Field; 10] = [
...
-961435057317293580094826482786572873533235701183329831124091847635547871092,
- -961435057317293580094826482786572873533235701183329831124091847635547871092
+ <unique_tenth_pedersen_hash>
];
Updates

Lead Judging Commences

s3mvl4d Lead Judge
20 days ago
s3mvl4d Lead Judge 18 days ago
Submission Judgement Published
Validated
Assigned finding tags:

broken double-claim protection

In `claim()`, the guard uses `claimed[_treasureHash]`, where `_treasureHash` is an immutable state variable that is never initialized to the caller-supplied treasure identifier, while the contract later marks `claimed[treasureHash] = true` using the function argument instead. As a result, the duplicate-claim check and the state update are performed against different keys, which means a previously claimed treasure is not actually blocked from being claimed again with the same valid proof and `treasureHash`. This breaks a core invariant of the protocol described in the README, namely, that each treasure can only be redeemed once, and allows one valid treasure/proof pair to be reused to drain rewards repeatedly until either the `MAX_TREASURES` cap or the contract balance is exhausted.

unclaimable treasure / bricked withdraw path

The issue stems from a mismatch between the circuit and the contract’s economic assumptions: the Solidity contract is configured for `MAX_TREASURES = 10` and only allows the owner to call `withdraw()` once `claimsCount >= MAX_TREASURES`, while the Noir circuit’s baked-in `ALLOWED_TREASURE_HASHES` array does not actually contain ten distinct treasures because one hash is duplicated and another expected hash is missing. As a result, under the intended one-claim-per-treasure design described in the README, there are only nine uniquely claimable treasures even though the system is funded and accounted as if ten rewards can be legitimately redeemed. That creates two linked consequences from the same root cause: first, one treasure is effectively unclaimable because no valid proof can ever be generated for the missing allowed hash, and second, the normal “hunt over” withdrawal path becomes bricked because honest participants can never reach ten legitimate unique claims, leaving the post-hunt fund recovery logic via `withdraw` function permanently unreachable. The owner can still intervene through the emergency path.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!