SNARKeling Treasure Hunt

First Flight #59
Beginner FriendlyGameFiFoundry
100 EXP
Submission Details
Impact: high
Likelihood: high

Typo checking _treasureHash bypasses replay protection and allows total contract drainage

Author Revealed upon completion

Root + Impact

Description

  • The contract is designed to prevent replay attacks by checking if a treasure hash has already been claimed through theclaimedmapping before processing a new claim

  • The critical bug occurs because the replay protection check uses the wrong variable - it checks claimed[_treasureHash]

    (an uninitialized immutable variable that always equalsbytes32(0)) instead ofclaimed[treasureHash]

    (the function parameter containing the actual treasure hash being claimed)

/ Root cause in the codebase
bytes32 private immutable _treasureHash; // @> Never initialized in constructor, defaults to bytes32(0)
function claim(bytes calldata proof, bytes32 treasureHash, address payable recipient) external nonReentrant() {
...
if (claimed[_treasureHash]) revert AlreadyClaimed(treasureHash); // @> Bug: checks wrong variable
...
_markClaimed(treasureHash); // @> Correctly marks the parameter, but guard never checks it
...
}

Risk

Likelihood: HIGH

  • This will occur every time a valid proof is submitted multiple times

  • The bug is deterministic and will always bypass replay protection since

    claimed[bytes32(0)]

    remains false

Impact: CRITICAL

  • Allows draining the entire 100 ETH prize pool with a single valid proof

  • Attacker can replay the same proof up to 10 times (limited only by MAX_TREASURES)

  • Each replay transfers 10 ETH to the attacker's specified recipient address

  • Total loss: up to 100 ETH from replay exploitation

Proof of Concept

// The uninitialized immutable variable defaults to bytes32(0)
bytes32 private immutable _treasureHash; // Never assigned, always 0x0
function claim(bytes calldata proof, bytes32 treasureHash, address payable recipient) external nonReentrant() {
...
// Bug: This always checks claimed[0x0] instead of claimed[treasureHash]
if (claimed[_treasureHash]) revert AlreadyClaimed(treasureHash);
// The actual treasureHash is marked correctly, but the guard above never checks it
_markClaimed(treasureHash);
...
}
// Attack sequence:
// 1. Attacker obtains one valid proof for treasureHash X
// 2. Calls claim(proof, X, attackerWallet) - succeeds, receives 10 ETH
// 3. Calls claim(proof, X, attackerWallet) again - succeeds again (guard checks wrong slot)
// 4. Repeats until claimsCount reaches 10, draining all 100 ETH

Recommended Mitigation

Remove the unused

_treasureHash

immutable variable and fix the replay protection check to use the correct parameter:

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