SNARKeling Treasure Hunt

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

Constructor does not enforce the `REWARD * MAX_TREASURES = 100 ETH` funding invariant, letting an under-funded deployment silently strand claimants

Author Revealed upon completion

Scope: contracts/src/TreasureHunt.sol, contracts/scripts/Deploy.s.sol

Root + Impact

Description

The contract documents that it "should be funded with enough ETH to cover all rewards (default deployment flow uses 100 ether)" and the in-scope Deploy.s.sol defaults INITIAL_FUNDING to 100 ether. But the constructor itself accepts ANY msg.value, including zero:

// contracts/src/TreasureHunt.sol (constructor, lines 67-75)
constructor(address _verifier) payable {
if (_verifier == address(0)) revert InvalidVerifier();
owner = msg.sender;
verifier = IVerifier(_verifier);
paused = false;
// Owner should fund 100 ETH at deployment (10 treasures × 10 ETH).
}
// contracts/scripts/Deploy.s.sol (run, lines 43-76)
uint256 initialFunding = vm.envOr("INITIAL_FUNDING", DEFAULT_INITIAL_FUNDING);
...
hunt = new TreasureHunt{value: initialFunding}(address(verifier));

If the deployer mis-sets INITIAL_FUNDING (e.g. forgets the env var, sets a wrong decimal count, or fat-fingers a small value), the contract deploys with insufficient ETH to pay all 10 rewards. Later claim() calls will succeed until the balance hits zero, then revert with NotEnoughFunds() — stranding any legitimate claimant whose slot happens to fall after the pool runs dry.

Also: since MAX_TREASURES is 10 but only 9 unique hashes are provable (Finding 2), the hunt cannot complete honestly. Combined with under-funding, this makes withdraw() even more unreachable.

Risk

Likelihood: MEDIUM

  • Requires owner mistake on deployment. Owner is trusted, but trust ≠ perfection; the constructor provides a cheap guardrail that this one doesn't use.

  • The README and comments set the expectation "100 ETH"; the code doesn't enforce it.

Impact: LOW

  • No direct theft. Later claimants DoS'd and some find-the-treasure work is stranded off-chain. Owner can reclaim unused ETH via emergencyWithdraw (requires pause), so no total-loss scenario.

  • Damages game UX / protocol reputation because discrepancies between "I have the secret" and "I got paid" look like a bug to the claimant.

Proof of Concept

// Deploy with only 5 ETH (wrong env var, or owner slip).
vm.prank(owner);
underfundedHunt = new TreasureHunt{value: 5 ether}(address(new HonkVerifier()));
// A legitimate finder of treasure #1 tries to claim...
(bytes memory proof, bytes32 treasureHash, address payable recipient) = _loadFixture();
vm.prank(participant);
vm.expectRevert(abi.encodeWithSelector(TreasureHunt.NotEnoughFunds.selector));
underfundedHunt.claim(proof, treasureHash, recipient);

(See testClaimWhenNotEnoughFundsFails in the existing test suite, which confirms this path reverts — demonstrating the failure mode without a guard at deployment.)

Recommended Mitigation

Enforce the funding invariant in the constructor:

constructor(address _verifier) payable {
if (_verifier == address(0)) revert InvalidVerifier();
+ require(msg.value == REWARD * MAX_TREASURES, "UNDERFUNDED_AT_DEPLOY");
owner = msg.sender;
verifier = IVerifier(_verifier);
paused = false;
}

(Or loosen to msg.value >= REWARD * MAX_TREASURES if overfunding is acceptable.)

Alternatively, if partial-funding deployments are a legitimate use case, add a corresponding DEPLOYED_UNDERFUNDED event so off-chain tooling surfaces the warning immediately.

Disclosure

This finding was identified with the assistance of an autonomous AI auditor (Anthropic Claude). Surfaced during the Claude + GPT brainstorm review of the initial 6-finding batch.

Support

FAQs

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

Give us feedback!