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

[H-1] Mismatched variable in claimed mapping allows unlimited reward claiming and full contract drain

ezenwachidiebere

Root + Impact

Description

  • The claim function is intended to prevent users from claiming the same treasure multiple times by checking a claimed mapping. Under normal behavior, a successful claim should mark the specific treasure hash as "true" in the mapping and revert any future attempts using that same hash.


  • However, a critical logic error exists where the duplicate check and the storage update use different variables. The contract checks _treasureHash (an incorrect or undefined identifier) but updates the mapping using treasureHash (the actual function parameter).Describe the normal behavior in one or more sentences


// @> The check reads from the wrong mapping key (_treasureHash)
if (claimed[_treasureHash]) revert AlreadyClaimed(treasureHash);
​
// @> The update writes to the correct key (treasureHash)
_markClaimed(treasureHash);h @> marks to highlight the relevant section

Risk

Likelihood:

  • Reason 1 This vulnerability is present in the core claim logic and is triggered every time a user attempts to claim a reward.

  • Reason 2 The duplicate check always evaluates to false because the slot being checked is never the slot being written to.

Impact:

  • Impact 1 An attacker can completely drain the contract's ETH balance with only one legitimate treasure discovery.

  • Impact 2 A single valid ZK proof can be reused infinitely by a single participant.

Proof of Concept

function test_replayAttack() public {
address attacker = makeAddr("attacker");
// Standard setup with one valid proof and its corresponding hash
bytes memory validProof = new bytes(0);
bytes32 treasureHash = bytes32(0);
​
uint256 contractBalanceBefore = address(treasureHunt).balance;
​
// First claim - legitimate
vm.prank(attacker);
treasureHunt.claim(validProof, treasureHash, payable(attacker));
​
// Second claim - the replay attack
// This should revert but passes due to the variable mismatch in the mapping check
vm.prank(attacker);
treasureHunt.claim(validProof, treasureHash, payable(attacker));
​
// Assert that the contract has been billed twice for the same treasure discovery
assertEq(address(treasureHunt).balance, contractBalanceBefore - 2 ether);
}
​
POC EXPLANATION
Double Claim Vulnerability.
i initialize the TreasureHunt contract and simulate a legitimate first claim by an attacker using a valid ZK proof.
Immediately after, the attacker calls claim() a second time with the exact same parameters.
Under secure conditions, the second call should revert with AlreadyClaimed. However, because the contract checks the wrong mapping key (_treasureHash instead of the actual treasureHash), the check passes.
The test uses an assertEq to prove that the contract's balance has decreased by twice the reward amount, confirming that the funds were successfully drained via replay.

Recommended Mitigation

- if (claimed[_treasureHash]) revert AlreadyClaimed(treasureHash);
+ if (claimed[treasureHash]) revert AlreadyClaimed(treasureHash);
_markClaimed(treasureHash);
​
MITIGATION EXPLANATION
To resolve this vulnerability, the contract must ensure that the identifier used to verify the claim status is the same identifier used to mark the claim as completed.
The fix involves replacing the incorrect identifier _treasureHash in the if statement with the function parameter treasureHash. This ensures the contract correctly points to the storage slot that is updated during the first successful claim, effectively enabling the intended replay protection.
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!