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

[H-01] Logic error in `claim()` leads to permanent state corruption

jantolivers

Description

  • The claim function is designed to prevent double-claiming by marking each found treasureHash as true in the claimed mapping.

  • During my review, I identified a critical variable mismatch: the contract updates a global, uninitialized state variable _treasureHash instead of the function's input parameter treasureHash. Since _treasureHash defaults to bytes32(0), the mapping fails to track actual treasure discoveries, leading to a broken game state where the real hashes remain "unclaimed" while the zero-hash is erroneously blocked.

Solidity

function claim(bytes calldata proof, bytes32 treasureHash, address payable recipient) external nonReentrant() {
if (isEnded) revert HuntEnded();
if (claimed[treasureHash]) revert AlreadyClaimed(treasureHash);
​
// ... ZK verification ...
​
@> claimed[_treasureHash] = true; // Error: Updates global empty variable instead of input
}
​

Risk

Likelihood:

  • This is a 100% reproducible bug that triggers on the very first successful claim attempt by any user.

Impact:

  • The contract state becomes inconsistent immediately. Multiple users can technically claim the same treasure because the mapping never "locks" the actual treasureHash used in the proof.

Proof of Concept

The following Foundry test demonstrates that after a successful claim, the intended hash remains available for reuse while the uninitialized global variable is the one marked as claimed.

Solidity

function test_StateCorruptionPoC() public {
bytes32 actualTreasure = keccak256("treasure_1");
bytes memory proof = "mock_proof";
​
// Simulate a successful claim
vm.prank(address(0x1));
hunt.claim(proof, actualTreasure, payable(address(0x1)));
​
// LOGIC FAILURE: The mapping for the actual treasure is still FALSE
bool isStillAvailable = !hunt.isClaimed(actualTreasure);
assertTrue(isStillAvailable, "The treasure should have been marked as claimed but it is not");
​
// Side effect: The zero-hash is now claimed instead
assertTrue(hunt.isClaimed(bytes32(0)), "The uninitialized global variable was updated");
}
​

Recommended Mitigation

Update the mapping using the function parameter to ensure the state correctly reflects the treasure found.

Diff

- claimed[_treasureHash] = true;
+ claimed[treasureHash] = true;
Updates

Lead Judging Commences

s3mvl4d Lead Judge 18 days ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

state corruption

The bug in `claim()` does not corrupt contract state or brick future claims; it simply checks the wrong mapping key when enforcing uniqueness. The problematic line reads `if (claimed[_treasureHash]) revert AlreadyClaimed(treasureHash);`, where `_treasureHash` is a separate immutable state variable, while the actual state update later uses the caller-supplied argument through `_markClaimed(treasureHash)`, which sets `claimed[treasureHash] = true`. Since the read and the write are performed against different keys, the effect is a broken duplicate-claim guard, not a destructive state transition. After one treasure is claimed, the mapping entry for that specific `treasureHash` is updated correctly, and other participants can still claim different treasures because the contract has not globally invalidated the claimed mapping or otherwise poisoned shared state. The real consequence of the bug is that previously claimed treasures are not properly blocked from being claimed again, not that the contract becomes unusable for everyone else.

Support

FAQs

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

Give us feedback!