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

Uninitialized immutable field makes double-claim check ineffective

mnangatariel27

Root + Impact

Description

Line 35 declares the immutable field:

bytes32 private immutable _treasureHash;

But the constructor (lines 67-75) NEVER initializes it:

constructor(address _verifier) payable {
if (_verifier == address(0)) revert InvalidVerifier();
owner = msg.sender;
verifier = IVerifier(_verifier);
paused = false;
// _treasureHash is left uninitialized!
}

In Solidity, uninitialized immutable fields default to 0x0000...0000.
The double-claim check at line 88 becomes if (claimed[0x0000...0000]) revert —
this checks if 0x0 is claimed, but 0x0 is never marked as claimed.
Meanwhile, actual treasures ARE marked as claimed, but the check bypasses them.

This creates state inconsistency:

  • claimed[treasureHash] = true (marked correctly)

  • claimed[_treasureHash] = always false (check uses this wrong key)

Risk

Likelihood:

  • Triggered on every single claim() call from deployment

  • No special conditions needed — uninitialized immutable is permanent

Impact:

  • Double-claim check is permanently bypassed

  • Same treasure can be claimed unlimited times

  • All 100 ETH in the contract is at risk of full drainage

Proof of Concept

This test proves the state inconsistency. After a treasure is claimed,
claimed[treasureHash] is correctly true, but claimed[bytes32(0)]
which is what line 88 actually checks — remains false, so the next
claim for the same treasure still succeeds.

function testVerify_StateInconsistency() public {
// Before claim
assertFalse(hunt.claimed(treasureHash));
assertFalse(hunt.claimed(bytes32(0))); // _treasureHash defaults to 0x0
​
// Claim the treasure
vm.prank(participant);
hunt.claim(proof, treasureHash, payable(recipient));
​
// After claim
assertTrue(hunt.claimed(treasureHash)); // Correctly marked
assertFalse(hunt.claimed(bytes32(0))); // But this is what's actually checked!
}

Recommended Mitigation

The field is fundamentally broken — never initialized, never set, and
only used in the broken check. Deleting it is the cleanest fix.

- bytes32 private immutable _treasureHash;

Option 2 would be to initialize it in the constructor, but the field
has no valid purpose so deletion is recommended.

Updates

Lead Judging Commences

s3mvl4d Lead Judge about 1 month 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!