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

Double claim does not revert due to wrong variable in claimed lookup

fredo182

Double claim does not revert due to wrong variable in claimed lookup

Description

The claim function is expected to prevent the same treasure from being claimed more than once by checking whether it has already been claimed and revert with AlreadyClaimed if the treasure has already been collected.

However, the duplicate claim validation checks claimed[_treasureHash] using the immutable state variable _treasureHash instead of checking against the actual treasureHash function parameter. As a result, the lookup always reads an unrelated mapping slot, allowing the same treasure to be claimed up to 10 times.

function claim(bytes calldata proof, bytes32 treasureHash, address payable recipient) external nonReentrant {
// ...
// @audit `_treasureHash` is the uninitialized immutable, not the `treasureHash` parameter
@> if (claimed[_treasureHash]) revert AlreadyClaimed(treasureHash);
// ...
_markClaimed(treasureHash); // correctly uses the parameter
}

Risk

Likelihood: High

Every valid proof can be replayed by any participant up to 10 times, since the validation never prevents a second claim for the same treasureHash.

Impact: High

An attacker can drain the entire contract balance by replaying a single valid proof.

Proof of Concept

function test_clamDoubleSpend_reverts() public {
(bytes memory proof, bytes32 treasureHash, address payable recipient) = _loadFixture();
​
vm.startPrank(participant);
hunt.claim(proof, treasureHash, recipient);
​
// This should revert with AlreadyClaimed but does NOT
vm.expectRevert(TreasureHunt.AlreadyClaimed.selector);
hunt.claim(proof, treasureHash, recipient);
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!