SNARKeling Treasure Hunt
First Flight #59
Beginner FriendlyGameFiFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Valid

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

copperbramble

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.

Updates

Lead Judging Commences

s3mvl4d Lead Judge 18 days ago
Submission Judgement Published
Validated
Assigned finding tags:

liquidity enforcement issues

The liquidity-enforcement issue arises because the protocol assumes the hunt will be funded with enough ETH to cover all rewards, but the contract itself does not actively enforce that invariant at deployment time. The constructor accepts arbitrary `msg.value` and only validates the verifier address, even though the contract hardcodes a reward of 10 ether and a maximum of 10 treasures, implying an expected full funding target of 100 ether; the README likewise states that the contract is expected to be funded with enough ETH to cover all rewards and notes that the default deployment flow uses 100 ether. Although the deployment script sets `DEFAULT_INITIAL_FUNDING = 100 ether`, it also allows that amount to be overridden via `vm.envOr("INITIAL_FUNDING", DEFAULT_INITIAL_FUNDING)`, and the only post-deployment balance check is that the contract balance equals the chosen `initialFunding`, not that the chosen amount is actually sufficient to fund the full hunt. As a result, the system can be deployed in an underfunded state after which otherwise valid claims will begin reverting with `NotEnoughFunds()` once the balance drops below a single reward payment.

Support

FAQs

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

Give us feedback!