SNARKeling Treasure Hunt

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

`ALLOWED_TREASURE_HASHES[8]` duplicates index 9; treasure 9 is unreachable and residual prize is stranded

Description

Indices 8 and 9 of ALLOWED_TREASURE_HASHES in circuits/src/main.nr are byte-identical. Index 8 should be pedersen_hash([9]) per Deploy.s.sol:25, but holds pedersen_hash([10]), duplicating index 9. is_allowed(pedersen_hash([9])) therefore returns false, so no valid proof can be generated for secret 9, and honest play caps at nine distinct claims. With the separate dedup bug patched, withdraw() at line 224 (claimsCount >= MAX_TREASURES) becomes unreachable and 10 ETH plus any donations are stranded.

// circuits/src/main.nr:64-65
-961435057317293580094826482786572873533235701183329831124091847635547871092, // index 8, WRONG
-961435057317293580094826482786572873533235701183329831124091847635547871092 // index 9
// contracts/scripts/Deploy.s.sol:25 (intended index 8)
-4417726114039171734934559783368726413190541565291523767661452385022043124552

The test file already works around the bug: circuits/src/tests.nr:30 uses [1, 2, 3, 4, 5, 6, 7, 8, 10, 10] (no 9), acknowledging that secret 9 cannot be proven.

Risk

Likelihood: certain. Activates whenever the finder of treasure 9 attempts to claim, or the owner attempts non-emergency withdrawal. Impact: (a) finder of treasure 9 receives nothing; (b) 10 ETH of residual prize plus any donations stuck; (c) owner can only recover via pause() + emergencyWithdraw, which cancels the hunt for remaining participants.

Proof of Concept

Noir test proving that secret 9's intended hash is not accepted (add to circuits/src/tests.nr):

#[test(should_fail)]
fn test_secret_9_unreachable() {
let treasure_hash: Field = -4417726114039171734934559783368726413190541565291523767661452385022043124552;
main(9, treasure_hash, 1); // fails because is_allowed(treasure_hash) == false
}

Distinctness check that would fail on current code:

#[test]
fn test_allowed_hashes_are_pairwise_distinct() {
for i in 0..10 {
for j in (i + 1)..10 {
assert(ALLOWED_TREASURE_HASHES[i] != ALLOWED_TREASURE_HASHES[j]);
}
}
}

Solidity-side impact once dedup is patched:

// after nine honest claims (secrets {1..8, 10}):
assertEq(hunt.claimsCount(), 9);
vm.expectRevert(bytes("HUNT_NOT_OVER"));
hunt.withdraw(); // blocked forever; 10 ETH stranded

Recommended Mitigation

Replace the wrong value at circuits/src/main.nr:64:

- -961435057317293580094826482786572873533235701183329831124091847635547871092,
+ -4417726114039171734934559783368726413190541565291523767661452385022043124552,

Regenerate Verifier.sol via circuits/scripts/build.sh and redeploy. Fix the test fixture:

- let treasures: [Field; 10] = [1, 2, 3, 4, 5, 6, 7, 8, 10, 10];
+ let treasures: [Field; 10] = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10];

Add the pairwise-distinctness test above so future copy-paste errors fail at nargo test time.

Updates

Lead Judging Commences

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

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!