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

Wrong variable in `claimed` check lets attacker drain all funds

Author Revealed upon completion

Description

  • When a user calls claim(), the contract verifies a ZK proof and if valid — sends 10 ETH to the recipient and marks the treasure as claimed so nobody can claim it again.

  • The problem is that the anti-double-claim check uses the wrong variable. It checks claimed[_treasureHash] where _treasureHash is an immutable field that is never set in the constructor, so it's always bytes32(0). But the actual write goes to claimed[treasureHash] (the parameter). So the check and the write are looking at completely different storage slots — the check never sees the write and always passes.

// _treasureHash is declared here but never assigned anywhere → always bytes32(0)
@> bytes32 private immutable _treasureHash;
​
function claim(bytes calldata proof, bytes32 treasureHash, address payable recipient) external {
...
@> if (claimed[_treasureHash]) revert AlreadyClaimed(treasureHash); // always false
...
_markClaimed(treasureHash); // writes to a different slot
(bool sent,) = recipient.call{value: REWARD}("");
}

Risk

Likelihood:

  • Anyone who gets a valid proof for any single treasure can replay the exact same call 10 times in a row — there's nothing stopping them.

  • No special access or setup needed, just a normal wallet and one proof generated off-chain.

Impact:

  • The attacker drains all 100 ETH from the contract in 10 transactions.

  • All claim slots get used up, so no other player can ever claim any treasure.

Proof of Concept

The attacker generates one valid proof for any treasure, then replays the same (proof, treasureHash, recipient) triple 10 times. Each call passes the claimed[bytes32(0)] check (always false) and transfers 10 ETH, draining the full contract balance.

function test_drainWithSingleProof() public {
bytes memory validProof = ...;
bytes32 treasureHash = ...;
address payable attacker = payable(makeAddr("attacker"));
address payable recipient = payable(makeAddr("recipient"));
​
for (uint256 i = 0; i < hunt.MAX_TREASURES(); i++) {
vm.prank(attacker);
hunt.claim(validProof, treasureHash, recipient);
}
​
assertEq(address(hunt).balance, 0);
assertEq(hunt.getClaimsCount(), 10);
}

Recommended Mitigation

Just remove the unused _treasureHash field and fix the check to use the function parameter:

- bytes32 private immutable _treasureHash;
​
- if (claimed[_treasureHash]) revert AlreadyClaimed(treasureHash);
+ if (claimed[treasureHash]) revert AlreadyClaimed(treasureHash);

Support

FAQs

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

Give us feedback!