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

Logic eror in claim function uses uninitialized immutable variable instead of parameter

Author Revealed upon completion

[H-01] Incorrect variable check in claim() bricks the contract after the first claim

Description

A critical logic vulnerability exists in the TreasureHunt::claim function. The contract fails to correctly verify the claim status of specific treasures because it references an uninitialized immutable state variable instead of the function's input parameter.

In the contract state, _treasureHash is declared as an immutable variable but is never assigned a value in the constructor:

bytes32 private immutable _treasureHash; // Defaults to 0x0...
​

Inside the claim function, the validation check is implemented as follows:

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

The issue is that the contract consistently checks the mapping for the default value 0x0 (the uninitialized state of _treasureHash), completely ignoring the unique treasureHash provided by the user in the function arguments.

Risk Assessment

Likelihood: High. The bug is triggered by the normal and intended use of the contract. The very first claim will likely pollute the state check for all subsequent users.

Impact: High. 90% of the protocol's funds (90 ETH) become inaccessible. The core business logic of the "Treasure Hunt" is destroyed as no further rewards can be distributed, leading to a complete failure of the smart contract's purpose.

Impact

This is a High severity issue as it leads to a permanent Denial of Service (DoS) of the protocol's core functionality after a single successful claim.

Once the first user successfully claims a treasure, the logic effectively "pollutes" the global claim check.

Because the contract checks the static _treasureHash (0x0) rather than the input parameter, every subsequent attempt to claim any other treasure will trigger the AlreadyClaimed revert.

This "bricks" the contract, making it impossible to distribute the remaining 9 rewards. Consequently, 90 ETH (90% of the total rewards) remains locked in the contract, defeating the purpose of the treasure hunt and the protocol's economic incentives.

Proof of Concept (PoC)

To verify this vulnerability, follow these steps:

  1. Go into test/TreasureHuntPoC.t.sol.

  2. Paste the following code into the file. This test simulates two different users trying to claim two different treasures.

Solidity

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.27;
​
import "forge-std/Test.sol";
import "../src/TreasureHunt.sol";
​
contract TreasureHuntPoC is Test {
TreasureHunt public hunt;
address public userA = address(0x1);
address public userB = address(0x2);
address public verifier = address(0x3);
​
function setUp() public {
// Deploy with 100 ETH to cover 10 rewards of 10 ETH
hunt = new TreasureHunt{value: 100 ether}(verifier);
vm.deal(userA, 1 ether);
vm.deal(userB, 1 ether);
}
​
function test_BrickedAfterFirstClaim() public {
bytes32 hashA = keccak256("Treasure_A");
bytes32 hashB = keccak256("Treasure_B");
bytes memory mockProof = "mock_proof";
​
// We mock the verifier to always return true for this test
vm.mockCall(verifier, abi.encodeWithSignature("verify(bytes,bytes32[])"), abi.encode(true));
// 1. User A claims Treasure A successfully
vm.prank(userA);
hunt.claim(mockProof, hashA, payable(address(0x88)));
​
// 2. User B tries to claim Treasure B (a completely different hash)
// This SHOULD work, but the contract checks 'claimed[_treasureHash]' (0x0)
// and reverts because the first claim already "polluted" the logic.
vm.prank(userB);
vm.expectRevert(abi.encodeWithSelector(TreasureHunt.AlreadyClaimed.selector, hashB));
hunt.claim(mockProof, hashB, payable(address(0x99)));
}
}
​

  1. Run the test using the following command in your terminal:

Bash

forge test --mt test_BrickedAfterFirstClaim -vvvv
​

The test will pass, confirming that the second claim reverts when it should have succeeded.

Recommended Mitigation

Update the validation logic to use thetreasureHash parameter provided in the function call:

- if (claimed[_treasureHash]) revert AlreadyClaimed(treasureHash);
+ if (claimed[treasureHash]) revert AlreadyClaimed(treasureHash);

The issue is that the contract consistently checks the mapping for the default value 0x0 (the uninitialized state of _treasureHash), completely ignoring the unique treasureHash provided by the user in the function arguments.

Support

FAQs

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

Give us feedback!