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

Broken duplicate-claim protection allows repeated claims for the same treasure

themilenkov

The contract intends to prevent multiple claims for the same treasureHash, but incorrectly checks a different storage key:

The contract checks whether a treasure has already been claimed using a different key than the one it writes to.

Instead of checking the user-supplied treasureHash, it checks _treasureHash, which is never initialized and defaults to bytes32(0).

// @> Reads wrong key: always bytes32(0)
if (claimed[_treasureHash]) revert AlreadyClaimed(treasureHash);
​
// ...
​
// @> Writes correct key: user-supplied treasureHash
claimed[treasureHash] = true;

Risk

Likelihood

  • The issue occurs on every call to claim() because _treasureHash is always bytes32(0)

Impact

  • Multiple rewards can be claimed for the same treasure

Proof of Concept

Replace testClaimDoubleSpendReverts() with:

function testDuplicateClaimAllowed() public {
(
bytes memory proof,
bytes32 treasureHash,
address payable recipient
) = _loadFixture();
​
vm.startPrank(participant);
​
// First claim succeeds
hunt.claim(proof, treasureHash, recipient);
​
uint256 beforeBal = recipient.balance;
​
// Second claim with same proof and hash SHOULD revert but DOES NOT
hunt.claim(proof, treasureHash, recipient);
​
// Balance increased again => duplicate claim succeeded
assertEq(recipient.balance, beforeBal + hunt.REWARD());
​
vm.stopPrank();
}

Recommended Mitigation

- 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!