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

Wrong Variable in Duplicate Claim Check Allows Multiple Claims

gz627

Root + Impact

Description

  • Each treasure reward can be claimed only once.

  • Multiple claims on the same Treasure are allowed due to the bug.

// @> contracts/src/TreasureHunt.sol Line 88
if (claimed[_treasureHash]) revert AlreadyClaimed(treasureHash); // BUG: uses _treasureHash

Risk

Likelihood:

  • Reason 1: Each claim will run this check.

Impact:

  • Impact 1: Line 88 checks claimed[_treasureHash] (an immutable storage variable that is never initialized, defaults to bytes32(0)) instead of claimed[treasureHash] (the function parameter). This causes the duplicate check to always validate the same storage slot regardless of which treasure is being claimed.


Proof of Concept

A valid treasure discovery can claim multiple times.

Game starts...
A lucky participent discovers a Treasure and claimes the reward with a `treasureHash`;
The claim succeeds;
The participent claims the reward again and again until the reward pool empty.

Recommended Mitigation

Replace the _treasureHashwith treasureHashin claim function: TreasureHunt.claim().

// function: TreasureHunt.claim() Line 88
function claim(bytes calldata proof, bytes32 treasureHash, address payable recipient) external nonReentrant() {
if (paused) revert ContractPaused();
if (address(this).balance < REWARD) revert NotEnoughFunds();
if (recipient == address(0) || recipient == address(this) || recipient == owner || recipient == msg.sender) revert InvalidRecipient();
if (claimsCount >= MAX_TREASURES) revert AllTreasuresClaimed();
- if (claimed[_treasureHash]) revert AlreadyClaimed(treasureHash);
+ if (claimed[treasureHash]) revert AlreadyClaimed(treasureHash);
if (msg.sender == owner) revert OwnerCannotClaim();
​
​
// Public inputs must match Noir circuit order:
// treasure_hash, recipient (recipient encoded into 160-bit integer in a Field).
bytes32[] memory publicInputs = new bytes32[](2);
publicInputs[0] = treasureHash;
publicInputs[1] = bytes32(uint256(uint160(address(recipient))));
​
// Verify proof against the public inputs.
// If valid, transfer the reward and mark the treasure as claimed.
bool ok = verifier.verify(proof, publicInputs);
if (!ok) revert InvalidProof();
​
_incrementClaimsCount();
_markClaimed(treasureHash);
​
​
(bool sent, ) = recipient.call{value: REWARD}("");
require(sent, "ETH_TRANSFER_FAILED");
​
emit Claimed(treasureHash, msg.sender);
}
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!