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