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

claim() checks wrong variable for duplicate claims, allowing the same treasure to be claimed multiple times

0xkhlo

Root + Impact: contracts/src/TreasureHunt.sol -> claimed[_treasureHash] checks the immutable constructor value instead of the caller-supplied treasureHash parameter

Description

  • The contract maintains a claimed mapping to prevent the same treasure from being claimed twice. Each claim should check and mark the caller-supplied treasureHash parameter.

    The duplicate-claim guard reads from claimed[_treasureHash] where _treasureHash is a separate immutable set at constructor time, but writes to claimed[treasureHash] where treasureHash is the caller-supplied parameter. These are two different storage slots, so the guard never blocks replay for any treasure hash other than the one that happens to match the constructor value.

function claim(bytes calldata proof, bytes32 treasureHash, address payable recipient) external nonReentrant() {
// ...
// @> reads from _treasureHash (immutable set in constructor)
if (claimed[_treasureHash]) revert AlreadyClaimed(treasureHash);
// ...
// @> writes to treasureHash (caller-supplied parameter)
_markClaimed(treasureHash);
}

Risk

Likelihood:

  • Any attacker who obtains a valid proof including a legitimate finder, immediately notices the same proof works again on the second call.

  • The exploit requires only a working proof and a loop, executable in a single script with no advanced knowledge

Impact:

  • A single valid proof is replayed indefinitely until the contract balance drops below 10 ETH, draining up to 100 ETH

claimsCount increments on every successful replay, corrupting the hunt state and potentially blocking legitimate claimants

Proof of Concept

// Attacker script
contract Drainer {
function drain(
address hunt,
bytes calldata proof,
bytes32 treasureHash,
address[] calldata recipients
) external {
for (uint i = 0; i < recipients.length; i++) {
// same proof works every time because claimed[_treasureHash]
// is never set to true for caller-supplied treasureHash
ITreasureHunt(hunt).claim(proof, treasureHash, recipients[i]);
}
}
}

Recommended Mitigation

function claim(bytes calldata proof, bytes32 treasureHash, address payable recipient) external nonReentrant() {
// ...
// @> check the caller-supplied treasureHash, not the immutable _treasureHash
- if (claimed[_treasureHash]) revert AlreadyClaimed(treasureHash);
+ if (claimed[treasureHash]) revert AlreadyClaimed(treasureHash);
// ...
}
​
// Also remove the unused _treasureHash immutable from the contract entirely
- bytes32 private immutable _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!