SNARKeling Treasure Hunt

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

Duplicate Hash Constants in Noir Circuit Leads to Permanent Fund Lock.

Root + Impact

Description

Root Cause

The Noir circuit in main.nr contains duplicate entries in the ALLOWED_TREASURE_HASHES array (indices 8 and 9 share the same hash value). This creates a deadlock where only 9 unique treasures can be claimed, permanently locking funds.

// Baked-in set of 10 allowed treasure hashes (pedersen hashes).
global ALLOWED_TREASURE_HASHES: [Field; 10] = [
1505662313093145631275418581390771847921541863527840230091007112166041775502,
-7876059170207639417138377068663245559360606207000570753582208706879316183353,
-5602859741022561807370900516277986970516538128871954257532197637239594541050,
2256689276847399345359792277406644462014723416398290212952821205940959307205,
10311210168613568792124008431580767227982446451742366771285792060556636004770,
-5697637861416433807484703347699404695743570043365849280798663758395067508,
-2009295789879562882359281321158573810642695913475210803991480097462832104806,
8931814952839857299896840311953754931787080333405300398787637512717059406908,
//@> -961435057317293580094826482786572873533235701183329831124091847635547871092,
//@> -961435057317293580094826482786572873533235701183329831124091847635547871092
];

Risk

Likelihood: High

Impact: High

Protocol Deadlock: The duplicate hash in Noir ensures the hunt can never be "completed" (9/10), which permanently bricks the withdraw() function for the owner, locking any remaining funds forever.

Proof of Concept

This PoC demonstrates the Protocol Deadlock where duplicate hashes in the Noir circuit prevent reaching the 10th claim.

Permanent Fund Lock (Duplicate Logic):

function test_LockedFundsDueToDuplicateHash() public {
(bytes memory proof, bytes32 treasureHash, address payable recipient) = _loadFixture();
vm.prank(participant);
hunt.claim(proof, treasureHash, recipient);
assertTrue(hunt.claimed(treasureHash));
assertEq(hunt.claimsCount(), 1);
vm.prank(participant);
vm.expectRevert(abi.encodeWithSelector(TreasureHunt.AlreadyClaimed.selector, treasureHash));
hunt.claim(proof, treasureHash, recipient);
vm.prank(owner);
vm.expectRevert("HUNT_NOT_OVER");
hunt.withdraw();
assertGe(address(hunt).balance, 10 ether);
}

Recommended Mitigation

The fix requires a two-step approach to restore both security and functionality.

  1. Noir Fix: Replace the duplicate hash at index 9 in ALLOWED_TREASURE_HASHES with a unique Pedersen hash. This ensures all 10 allowed hashes are distinct and allows the hunt to complete.

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!