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

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

blackburn

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 {
Updates

Lead Judging Commences

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

broken access control in withdraw()

The `withdraw()` function is intended as an owner-only post-hunt recovery function, but the implementation does not actually enforce any ownership check before transferring the full remaining balance to owner. The function only requires that `claimsCount >= MAX_TREASURES` and that the contract balance is nonzero, after which it sends all ETH to the stored owner address regardless of who called the function. Therefore, the access control on the function itself is incomplete because any external account can trigger the withdrawal path once the hunt is considered over.

Support

FAQs

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

Give us feedback!