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

Use of uninitialized_treasureHash prevents all claims and breaks state tracking

erzakelmendi7

Root + Impact

The contract defines a private immutable variable _treasureHash which is never initialized in the constructor. In solidity, uninitialized bytes32 variables default to 0x0.

Description

  • In a standard smart contract design, the claim function should verify that the specific treasure being submitted hasn't been previously claimed by checking its unique hash against the claimed mapping. This ensures that each of the ten individual reward can be distributed to ten different successful participants, while preventing any single treasure from being claimed multiple times.

  • The specific issue is that the contract checks a private, uninitialized state variable(_treasureHash) instead of the user-provided treasureHash parameter to determine if a reward has been claimed. Because uninitialized bytes32 variables default to 0x0, the contract tracks every single claim againts this same empty value, causing the logic to fail after the first successful transaction.

88 if (claimed[_treasureHash]) revert AlreadyClaimed(treasureHash);

Risk

Likelihood:

  • Reason 1 - The private immutable variable _treasureHash defaults to 0x0 due to a lack of initialization in the constructor.

  • Reason 2 - The claim function logic points specifically to this uninitialized variable instead of the function parameter, ensuring the error triggers during every execution.

Impact:

  • Impact 1 - The contract permanently locks all remaining rewards after the first successful claim, as the claimed[0x0] state becomes true for all subsequent calls.

    • Impact 2 - Nine out of ten treasures (90 ETH) become fundamently unclaimable, resulting in a near-total loss of protocol functionality.

Proof of Concept

"The following scenario demonstrates how an uninitialized state variable defaults to address zero, causing a permanent denial of service after the first successful claim."
The vulnerability can be demonstrated by following the state changes during two consecutive claim attempts:
​
1. Initial State: The contract is deployed. _treasureHash is uninitialized and therefore equals 0x0. The mapping claimed[0x0] is false.
​
2. First Valid Claim: A participant (User A) submits a valid ZK proof for Treasure_Hash_A.
​
• The contract checks if (claimed[_treasureHash]) which evaluates to if (claimed[0x0]).
​
• Since it is false, the transaction proceeds.
​
• The internal function _markClaimed(treasureHash) is called.
​
• Crucially: Even if _markClaimed updates the mapping for Treasure_Hash_A, the logic in claim (line 88) continues to only look at _treasureHash (0x0).
​
3. The "Brick" Event: During the first claim, if any part of the logic marks the 0x0 address as claimed (or if a user simply claims a treasure with a 0x0 hash), the state claimed[0x0] becomes true.
​
4. Second Claim Attempt: A participant (User B) submits a valid ZK proof for a completely different treasure, Treasure_Hash_B.
​
• The contract again checks if (claimed[_treasureHash]) which is if (claimed[0x0]).
​
• Because this is now true, the transaction reverts with AlreadyClaimed.
​
5. Result: No further treasures can ever be claimed, even though 9 treasures remain in the contract and the users have valid proofs.

Recommended Mitigation

"Update the logic to check the user-provided treasureHash parameter instead of the uninitialized _treasureHash state variable to ensure each claim is tracked correctly."
- remove this code
if (claimed[_treasureHash]) revert AlreadyClaimed(treasureHash);
+ add this code
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!