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

Typo checking _treasureHash bypasses replay protection and allows total contract drainage

bitguardx

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