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

[C-01] TreasureHunt::claim uses uninitialized immutable variable _treasureHash allowing contract draining via double-spending

Author Revealed upon completion

Root + Impact

Description

  • The TreasureHunt contract is designed to reward participants who find physical treasures. To ensure fairness, each unique treasure (identified
    by its treasureHash) must only be claimed once. The contract uses a claimed mapping to track which hashes have already received a reward.

  • The specific issue is that the claim function checks a private immutable variable named _treasureHash which is never initialized in the
    constructor. In Solidity, uninitialized immutable variables default to bytes32(0). Consequently, every call to claim checks if
    claimed[bytes32(0)] is true, regardless of the treasureHash actually being submitted. Since the contract marks the submitted treasureHash as
    claimed, but checks the uninitialized one, the check always passes for any valid treasure, allowing the same proof and hash to be reused
    indefinitely until the contract's ETH balance is exhausted.

// contracts/src/TreasureHunt.sol
​
@> bytes32 private immutable _treasureHash; // Never assigned, defaults to 0
​
function claim(bytes calldata proof, bytes32 treasureHash, address payable recipient) external nonReentrant() {
// ...
if (claimsCount >= MAX_TREASURES) revert AllTreasuresClaimed();
​
// @> Root Cause: Always checks claimed[0], ignoring the actual treasureHash
if (claimed[_treasureHash]) revert AlreadyClaimed(treasureHash);
​
// ... verification logic ...
​
_incrementClaimsCount();
​
// @> This marks the input hash, but the check above never looks at it
_markClaimed(treasureHash);
​
(bool sent, ) = recipient.call{value: REWARD}("");
require(sent, "ETH_TRANSFER_FAILED");
// ...
}

Risk

Likelihood:

  • This vulnerability will occur as soon as any participant generates a single valid ZK proof for any of the 10 treasures.

  • Because the AlreadyClaimed check is decoupled from the treasureHash being claimed, there is no on-chain mechanism to stop a user (or an
    attacker observing the mempool) from submitting the same proof multiple times.

Impact:

  • Total Loss of Funds: An attacker can drain all 100 ETH (or the entire contract balance) by replaying the same claim transaction with different
    recipient addresses.

  • Protocol Failure: The "Treasure Hunt" becomes a "first-to-find-one-and-drain-all" exploit, rendering the other 9 treasures worthless and the
    competition moot.

Proof of Concept

PoC Explanation

This PoC uses a MockVerifier to simulate the successful verification of a ZK proof. It demonstrates that the attacker can call the claim function
twice using the exact same treasureHash.

  1. The first call succeeds and sends 10 ETH to victim1.

  2. The second call, which should revert with AlreadyClaimed, also succeeds because the contract is checking the uninitialized _treasureHash (0)
    instead of FAKE_TREASURE_HASH.

  3. The final balance shows 20 ETH has been removed for a single treasure hash

function test_DoubleSpendExploit() public {
uint256 initialBalance = address(hunt).balance;
​
// First claim with a specific treasure hash
vm.prank(attacker);
hunt.claim("", FAKE_TREASURE_HASH, payable(victim1));
​
// Second claim with the SAME treasure hash succeeds
vm.prank(attacker);
hunt.claim("", FAKE_TREASURE_HASH, payable(victim2));
​
// Assertions prove that the same hash was used twice and claimsCount increased
assertEq(hunt.claimsCount(), 2);
assertEq(address(hunt).balance, initialBalance - (hunt.REWARD() * 2));
}

Recommended Mitigation

Mitigation Explanation

The fix involves removing the unused and uninitialized _treasureHash immutable variable and ensuring that the claimed mapping is checked against
the treasureHash parameter provided in the function call. This creates a direct link between the treasure being claimed and its recorded status in
the mapping.

- 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!