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

Uninitialized Immutable _treasureHash Causes Wrong Mapping Key Same Treasure Claimable Unlimited Times

cybervenom

Root + Impact

Description

  • The contract tracks claimed treasures via mapping(bytes32 => bool) public claimed. When a participant submits a valid proof, _markClaimed(treasureHash) writes claimed[treasureHash] = true using the caller-supplied treasureHash parameter.

However, the duplicate-claim guard on line 88 reads from claimed[_treasureHash] — the immutable storage variable — which is declared but never assigned in the constructor, meaning it is permanently bytes32(0).

  • These two keys are completely independent. Unless a claimant submits treasureHash == bytes32(0), the guard claimed[_treasureHash] always evaluates to false, and the same treasure can be claimed again on every subsequent call.

// Root cause — two different keys used for write vs. read:
​
bytes32 private immutable _treasureHash; // @> Never assigned; always bytes32(0)
​
function claim(...) external nonReentrant() {
...
// @> Checks claimed[bytes32(0)] — ALWAYS false unless treasureHash param is 0
if (claimed[_treasureHash]) revert AlreadyClaimed(treasureHash);
...
_incrementClaimsCount();
// @> Writes claimed[treasureHash_param] — correct key, but never re-read above
_markClaimed(treasureHash);
...
}

Risk

Likelihood:

  • Any participant who obtains one valid proof for one treasure can re-submit it with the same treasureHash on every call; the guard never fires because the read key (_treasureHash) and the write key (treasureHash param) are always different.

No special tooling is needed — a simple loop of claim() calls suffices.

Impact:

  • A single valid proof drains the entire contract balance in up to 10 sequential transactions (bounded only by MAX_TREASURES), defrauding all other treasure finders of their rewards.

Legitimate treasures are never recorded as claimed, so the hunt's state is completely corrupted.

Proof of Concept

Attacker has one valid proof for treasureHash = X.

claimsCount starts at 0, contract holds 100 ETH.

for (uint i = 0; i < 10; i++) {
// Each iteration:
// claimed[bytes32(0)] == false → guard passes
// claimed[X] is set → never read back by guard
// claimsCount++
hunt.claim(proof, X, recipientAddress); // drains 10 ETH each time
}
// After 10 calls: attacker has received 100 ETH, contract is empty.
​

Recommended Mitigation

Replace _treasureHash (the immutable) with treasureHash (the parameter) in the duplicate-claim guard, and remove the unused immutable variable:

- remove this code
// Remove this entirely — it serves no purpose and is never initialized:
// bytes32 private immutable _treasureHash;
​
+ // Before (buggy):
if (claimed[_treasureHash]) revert AlreadyClaimed(treasureHash);
​
// After (fixed):
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!