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

[H-01] Broken claimed guard allows repeated claims and full contract drain

adcdiii

Root + Impact

Description

The normal behavior is one reward payout per unique treasureHash. After a successful claim, the same hash must be blocked forever.

The issue is that claim() checks claimed[_treasureHash] where _treasureHash is an uninitialized immutable (bytes32(0)), while the write path marks claimed[treasureHash]. Because the read and write keys differ, the same valid proof/hash can be replayed until MAX_TREASURES is reached.

// contracts/src/TreasureHunt.sol
bytes32 private immutable _treasureHash; // @> never assigned in constructor, defaults to bytes32(0)
​
function claim(bytes calldata proof, bytes32 treasureHash, address payable recipient) external nonReentrant() {
// ...
if (claimed[_treasureHash]) revert AlreadyClaimed(treasureHash); // @> wrong key checked
// ...
_markClaimed(treasureHash); // @> correct key written, but never checked above
}

Risk

Likelihood:

  • Any user with one valid proof can call claim() repeatedly with the same treasureHash and different recipients.

  • The exploit path is direct and requires no privileged role.

Impact:

  • Full 100 ETH pool can be drained using one unique treasure proof.

  • Legitimate winners are blocked once claimsCount reaches MAX_TREASURES.

Proof of Concept

Standalone reproduction:

  1. Deploy TreasureHunt with 100 ether.

  2. Use a verifier that returns true for a valid (proof, treasureHash, recipient) tuple.

  3. Call claim(proof, HASH_1, recipient1) once.

  4. Call claim(proof, HASH_1, recipient2) again with the same proof and same HASH_1.

  5. Repeat until 10 successful claims. Contract balance reaches zero while only one unique hash was used.

function test_A2_FullDrainWithSingleProof() public {
uint256 balBefore = address(hunt).balance;
assertEq(balBefore, 100 ether);
​
for (uint256 i = 0; i < 10; i++) {
address payable r = payable(address(uint160(0x9000 + i)));
vm.prank(ATTACKER);
hunt.claim(DUMMY_PROOF, HASH_1, r); // same proof/hash replay
}
​
assertEq(address(hunt).balance, 0, "BUG: contract fully drained with 1 unique hash");
}

Recommended Mitigation

Use the function claim(bytes calldata proof, bytes32 treasureHash, address payable recipient).

function claim(bytes calldata proof, bytes32 treasureHash, address payable recipient) external nonReentrant() {
- if (claimed[_treasureHash]) revert AlreadyClaimed(treasureHash);
+ if (claimed[treasureHash]) revert AlreadyClaimed(treasureHash);
}
​
- bytes32 private immutable _treasureHash;
+ // remove unused immutable
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!