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

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

Author Revealed upon completion

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

Support

FAQs

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

Give us feedback!