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

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

Author Revealed upon completion

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);
// ...
}

Support

FAQs

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

Give us feedback!