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

Broken Double-Spend Protection — Same Treasure Can Be Claimed Repeatedly

0xaljzoli

Finding 1: Broken Double-Spend Protection Allows Same Treasure to Be Claimed Repeatedly

Root + Impact

Wrong variable referenced in AlreadyClaimed check + total reward pool drain

Description

  • The claim() function is designed to prevent the same treasure from being claimed twice by checking the claimed mapping against a treasure hash. Once a treasure is claimed, subsequent claims for that hash should revert with AlreadyClaimed.

  • The AlreadyClaimed check queries the immutable state variable _treasureHash (which is never assigned in the constructor and defaults to bytes32(0)) instead of the function parameter treasureHash. Meanwhile, _markClaimed() correctly marks the function parameter treasureHash as claimed. This mismatch means the double-spend check always queries a different key than the one that was set, so it never detects a previously claimed treasure.

bytes32 private immutable _treasureHash; // @> never assigned in constructor, defaults to bytes32(0)
​
function claim(bytes calldata proof, bytes32 treasureHash, address payable recipient) external nonReentrant() {
// ...
if (claimed[_treasureHash]) revert AlreadyClaimed(treasureHash); // @> checks wrong key — always checks claimed[bytes32(0)]
// ...
_markClaimed(treasureHash); // @> marks the correct key claimed[treasureHash], but the check above never reads this
// ...
}

Risk

Likelihood:

  • A participant with a single valid ZK proof for any treasure calls claim() repeatedly up to 10 times (until claimsCount >= MAX_TREASURES), since the AlreadyClaimed guard never triggers

  • The ZK proof remains valid for the same treasureHash + recipient combination on every call, so no new proof generation is needed

Impact:

  • A single participant drains the entire 100 ETH reward pool by repeatedly claiming the same treasure

  • 9 other legitimate treasure finders receive no reward

  • The integrity of the treasure hunt is completely broken

Proof of Concept

function testDoubleSpendDrainsContract() public {
(
bytes memory proof,
bytes32 treasureHash,
address payable recipient
) = _loadFixture();
​
uint256 initialBalance = address(hunt).balance; // 100 ether
​
// Claim the same treasure 10 times with the same proof
for (uint256 i = 0; i < 10; i++) {
hunt.claim(proof, treasureHash, recipient);
}
​
// Contract is now drained
assertEq(address(hunt).balance, 0 ether);
// Recipient received 10 × 10 ETH = 100 ETH
assertEq(recipient.balance, INITIAL_PARTICIPANT_BALANCE + 100 ether);
}

Note: The existing test testClaimDoubleSpendReverts at line 134 has vm.expectRevert() commented out, confirming this behavior was observed but not fixed.

Recommended Mitigation

- bytes32 private immutable _treasureHash;
function claim(bytes calldata proof, bytes32 treasureHash, address payable recipient) external nonReentrant() {
// ...
- if (claimed[_treasureHash]) revert AlreadyClaimed(treasureHash);
+ if (claimed[treasureHash]) revert AlreadyClaimed(treasureHash);
// ...
}
Updates

Lead Judging Commences

s3mvl4d Lead Judge 18 days ago
Submission Judgement Published
Validated
Assigned finding tags:

broken double-claim protection

In `claim()`, the guard uses `claimed[_treasureHash]`, where `_treasureHash` is an immutable state variable that is never initialized to the caller-supplied treasure identifier, while the contract later marks `claimed[treasureHash] = true` using the function argument instead. As a result, the duplicate-claim check and the state update are performed against different keys, which means a previously claimed treasure is not actually blocked from being claimed again with the same valid proof and `treasureHash`. This breaks a core invariant of the protocol described in the README, namely, that each treasure can only be redeemed once, and allows one valid treasure/proof pair to be reused to drain rewards repeatedly until either the `MAX_TREASURES` cap or the contract balance is exhausted.

Support

FAQs

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

Give us feedback!