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

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

Author Revealed upon completion

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;

Support

FAQs

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

Give us feedback!