SNARKeling Treasure Hunt
First Flight #59
Beginner FriendlyGameFiFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Only 9 Unique Treasures Leading to Permanent Lock of the `withdraw` Function

victifa

Root + Impact

Description

In main.nr, the last two values of the hardcoded ALLOWED_TREASURE_HASHES array are identical. This means that, in reality, only 9 unique treasures can generate valid proofs. However, MAX_TREASURES is set to 10 in the contract, and the withdraw function requires:

require(claimsCount >= MAX_TREASURES, "HUNT_NOT_OVER");

This makes it impossible for the owner to ever use the withdraw function to retrieve unclaimed funds after the hunt is over.

Risk

Likelihood: High

  • This is a hardcoded logical error.

  • It occurs when 9 treasures are claimed and the last hunter cannot obtain a reward.

Impact: High

  • the owner will never be able to call withdraw() to extract remaining funds or unclaimed rewards, and can only rely on emergencyWithdraw to urgently send ETH to a specified address.

  • One of the two hunters that found the treasure with same treasureHash couldn't get the reward.

Proof of Concept

  1. Hunters find all possible treasures.

  2. 9 treasures are claimed; the last hunter cannot obtain a reward. claimsCount equals 9.

  3. After the hunt is over, the Owner attempts to withdraw the remaining 10 ETH. Calling withdraw() triggers require(9 >= 10), and the transaction fails.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.27;
​
import "forge-std/Test.sol";
import {TreasureHunt} from "../src/TreasureHunt.sol";
import {HonkVerifier} from "../src/Verifier.sol";
​
contract FundLockPoC is Test {
TreasureHunt hunt;
HonkVerifier verifier;
address owner = address(0xDEADBEEF);
​
// To facilitate handling negative Fields, they are converted to uint256
uint256[] public bakedInHashes;
​
function setUp() public {
bakedInHashes = new uint256[](10);
bakedInHashes[0] = 1505662313093145631275418581390771847921541863527840230091007112166041775502;
bakedInHashes[1] = uint256(-7876059170207639417138377068663245559360606207000570753582208706879316183353);
bakedInHashes[2] = uint256(-5602859741022561807370900516277986970516538128871954257532197637239594541050);
bakedInHashes[3] = 2256689276847399345359792277406644462014723416398290212952821205940959307205;
bakedInHashes[4] = 10311210168613568792124008431580767227982446451742366771285792060556636004770;
bakedInHashes[5] = uint256(-5697637861416433807484703347699404695743570043365849280798663758395067508);
bakedInHashes[6] = uint256(-2009295789879562882359281321158573810642695913475210803991480097462832104806);
bakedInHashes[7] = 8931814952839857299896840311953754931787080333405300398787637512717059406908;
bakedInHashes[8] = uint256(-961435057317293580094826482786572873533235701183329831124091847635547871092);
bakedInHashes[9] = uint256(-961435057317293580094826482786572873533235701183329831124091847635547871092);
​
vm.startPrank(owner);
verifier = new HonkVerifier();
​
hunt = new TreasureHunt{value: 100 ether}(address(verifier));
vm.stopPrank();
}
​
function test_FundLock_LogicBricked() public {
assertEq(bakedInHashes[8], bakedInHashes[9], "Circuit hashes 8 and 9 should be identical");
​
uint256 uniqueCount = 0;
for (uint i = 0; i < 10; i++) {
bytes32 h = bytes32(bakedInHashes[i]);
if (!hunt.isClaimed(h)) {
// Manually mark as claimed
// Calculate the storage slot for mapping(bytes32 => bool) claimed
bytes32 slot = keccak256(abi.encode(h, uint256(0)));
vm.store(address(hunt), slot, bytes32(uint256(1))); // Set to true
uniqueCount++;
}
}
​
// Manually sync the `claimsCount` variable
vm.store(address(hunt), bytes32(uint256(1)), bytes32(uniqueCount));
​
// Only 9 unique treasures have been claimed
assertEq(uniqueCount, 9, "There should only be 9 unique treasure hashes");
assertEq(hunt.getClaimsCount(), 9);
assertEq(address(hunt).balance, 10 ether, "10 ETH should remain in contract (for the duplicate hash)");
​
// Attempt to withdraw the remaining 10 ETH as Owner but failed
vm.prank(owner);
vm.expectRevert("HUNT_NOT_OVER");
hunt.withdraw();
}
}

Recommended Mitigation

  1. Update the Noir Circuit
    The root cause lies in the Noir circuit definition. Ensure that the ALLOWED_TREASURE_HASHES array in main.nr contains 10 unique Pedersen hashes. This ensures that there are 10 distinct valid secrets, allowing claimsCount to reach MAX_TREASURES.

global ALLOWED_TREASURE_HASHES: [Field; 10] = [
1505662313093145631275418581390771847921541863527840230091007112166041775502,
-7876059170207639417138377068663245559360606207000570753582208706879316183353,
-5602859741022561807370900516277986970516538128871954257532197637239594541050,
2256689276847399345359792277406644462014723416398290212952821205940959307205,
10311210168613568792124008431580767227982446451742366771285792060556636004770,
-5697637861416433807484703347699404695743570043365849280798663758395067508,
-2009295789879562882359281321158573810642695913475210803991480097462832104806,
8931814952839857299896840311953754931787080333405300398787637512717059406908,
-961435057317293580094826482786572873533235701183329831124091847635547871092,
- -961435057317293580094826482786572873533235701183329831124091847635547871092
+ hash10 that not equals -961435057317293580094826482786572873533235701183329831124091847635547871092
];
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!