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

`withdraw()` is unreachable under honest play once the dedup guard is fixed and the circuit duplicate is not

webrainsec

Description

TreasureHunt.withdraw() at line 224 gates on claimsCount >= MAX_TREASURES (ten). With the separate critical dedup bug patched so that claimed[h] actually fires, honest play cannot exceed nine successful claims because ALLOWED_TREASURE_HASHES has only nine distinct entries (see the separate finding on the index 8/9 duplicate). The 10 ETH residual prize plus any donations then sit stranded; the only recovery is pause() + emergencyWithdraw, which cancels the hunt.

function withdraw() external {
require(claimsCount >= MAX_TREASURES, "HUNT_NOT_OVER"); // unreachable if claimsCount caps at 9
...
}

Risk

Likelihood: certain the moment the dedup bug is patched in isolation. Impact: 10 ETH of prize plus any donations frozen; owner forced to cancel the hunt to recover. Dependent finding: masked as long as the dedup bug is live (a single replay reaches claimsCount == 10 trivially).

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.27;
​
import "forge-std/Test.sol";
import {TreasureHunt} from "../src/TreasureHunt.sol";
import {IVerifier} from "../src/Verifier.sol";
​
contract PoC_M01 is Test {
TreasureHunt hunt;
address mockVerifier = address(0xABCDEF);
bytes32[9] HASHES = [bytes32(uint256(1)), bytes32(uint256(2)), bytes32(uint256(3)),
bytes32(uint256(4)), bytes32(uint256(5)), bytes32(uint256(6)),
bytes32(uint256(7)), bytes32(uint256(8)), bytes32(uint256(10))];
​
function setUp() public {
vm.etch(mockVerifier, hex"00");
vm.deal(address(0xDEAD), 200 ether);
vm.prank(address(0xDEAD));
hunt = new TreasureHunt{value: 100 ether}(mockVerifier);
vm.mockCall(mockVerifier, abi.encodeWithSelector(IVerifier.verify.selector), abi.encode(true));
}
​
function test_withdraw_stranded_after_nine_honest_claims() public {
for (uint256 i = 0; i < 9; i++) {
vm.prank(address(uint160(0xB0B0 + i)));
hunt.claim(hex"de", HASHES[i], payable(address(uint160(0xA11CE0 + i))));
}
assertEq(hunt.claimsCount(), 9);
assertEq(address(hunt).balance, 10 ether);
vm.prank(address(0xDEAD));
vm.expectRevert(bytes("HUNT_NOT_OVER"));
hunt.withdraw(); // blocked forever
}
}

Recommended Mitigation

Primary fix: correct the circuit duplicate (separate finding) so ten distinct hashes exist and honest play can legitimately reach ten claims.

Defence in depth: loosen the gate to something the contract can observe, for example a grace period after the last claim, and add the missing onlyOwner:

function withdraw() external {
+ require(msg.sender == owner, "ONLY_OWNER");
- require(claimsCount >= MAX_TREASURES, "HUNT_NOT_OVER");
+ require(
+ claimsCount >= MAX_TREASURES ||
+ (lastClaimAt > 0 && block.timestamp > lastClaimAt + WITHDRAW_GRACE_PERIOD),
+ "HUNT_NOT_OVER"
+ );
...
}
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!