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

withdraw() missing onlyOwner modifier allows anyone to forcibly trigger owner's fund withdrawal

Author Revealed upon completion

Root + Impact

Description

  • The withdraw() function is intended to allow the owner to reclaim contract funds after all treasures have been claimed. However, the function has no access control — any address can call it once claimsCount >= MAX_TREASURES. Funds always transfer to the hardcoded owner address so direct theft is impossible, but any caller can force the withdrawal at an arbitrary time, removing the owner's ability to control timing.

// @> no onlyOwner modifier — callable by anyone
function withdraw() external {
require(claimsCount >= MAX_TREASURES, "HUNT_NOT_OVER");
uint256 balance = address(this).balance;
require(balance > 0, "NO_FUNDS_TO_WITHDRAW");
// @> funds always go to owner, but anyone triggered this
(bool sent, ) = owner.call{value: balance}("");
require(sent, "ETH_TRANSFER_FAILED");
}

Risk

Likelihood:

  • Condition claimsCount >= MAX_TREASURES is publicly readable — anyone can monitor it

  • No capital or special access required — a single transaction from any address

Impact:

  • Owner loses control over withdrawal timing — e.g. forced out during high gas periods

  • Combined with the replay attack, an attacker can max out claimsCount and immediately trigger withdraw(), closing the hunt programmatically

  • Contradicts explicit design intent — NatSpec and the github repo says "Allow the owner to withdraw" and the contract defines an OnlyOwnerCanFund error, yet the symmetric withdrawal has no guard

Proof of Concept

function testAnyoneCanTriggerWithdraw() public {
// Simulate hunt ending via replay (or legitimate claims)
vm.mockCall(address(hunt), abi.encodeWithSelector(hunt.claimsCount.selector), abi.encode(10));
​
address anyUser = makeAddr("anyUser");
uint256 ownerBalanceBefore = owner.balance;
​
vm.prank(anyUser); // not the owner
hunt.withdraw();
​
// Funds went to owner — but anyUser triggered it
assertGt(owner.balance, ownerBalanceBefore);
}

Recommended Mitigation

Just add the onlyOwner modifier to the function.

- function withdraw() external {
+ function withdraw() external onlyOwner {

Support

FAQs

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

Give us feedback!