SNARKeling Treasure Hunt

First Flight #59
Beginner FriendlyGameFiFoundry
100 EXP
View results
Submission Details
Severity: high
Valid

Broken claimed-check allows repeated payouts for the same treasure

Broken claimed-check allows repeated payouts for the same treasure

Description

  • The intended claim flow is: verify a proof for `(treasureHash, recipient)`, ensure the referenced treasure has not been claimed yet, then transfer exactly one fixed reward for that treasure.

  • The implementation checks `claimed[_treasureHash]` instead of `claimed[treasureHash]`. Since `_treasureHash` is never initialized in the constructor, the replay-prevention gate does not track the treasure being claimed. As a result, the same valid proof and the same `treasureHash` can be reused to receive multiple payouts.

function claim(bytes calldata proof, bytes32 treasureHash, address payable recipient) external nonReentrant {
// ...
// @> the duplicate-claim gate checks the wrong key
if (claimed[_treasureHash]) revert AlreadyClaimed(treasureHash);
// ...
bool ok = verifier.verify(proof, publicInputs);
if (!ok) revert InvalidProof();
_incrementClaimsCount();
// @> the contract marks the correct key only after the broken check
_markClaimed(treasureHash);
// ...
}

Risk

Likelihood: HIGH

  • Any user holding one valid proof can resubmit the same `(proof, treasureHash, recipient)` tuple multiple times because the duplicate-claim gate never checks the actual `treasureHash` being consumed.

  • The repository's own fixtures already contain a valid proof, and the PoC shows the issue is exploitable without special setup or malformed inputs.

Impact: HIGH

  • A single discovered treasure can be paid out more than once, breaking the protocol's core one-treasure/one-reward invariant.

  • Repeated submissions can drain the ETH reward pool that was meant to cover all treasures.

Proof of Concept

Paste this code and add to test file, then run it

function testPoCHigh_ReusedProofCanDrainRewardsForSameTreasure() public {
(bytes memory proof, bytes32 treasureHash, address payable recipient) = _loadFixture();
uint256 recipientBalanceBefore = recipient.balance;
uint256 contractBalanceBefore = address(hunt).balance;
vm.startPrank(participant);
hunt.claim(proof, treasureHash, recipient);
hunt.claim(proof, treasureHash, recipient);
vm.stopPrank();
assertEq(recipient.balance, recipientBalanceBefore + (2 * hunt.REWARD()));
assertEq(address(hunt).balance, contractBalanceBefore - (2 * hunt.REWARD()));
assertEq(hunt.claimsCount(), 2);
assertTrue(hunt.claimed(treasureHash));
}

Recommended Mitigation

To fix this just simple, we can remove the _treasureHash that imutable variable since we never use it, and change the check in claim() function with treasureHash from function params instead _treasureHash

- if (claimed[_treasureHash]) revert AlreadyClaimed(treasureHash);
+ if (claimed[treasureHash]) revert AlreadyClaimed(treasureHash);
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!