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

[M-1] Missing onlyOwner modifier on withdraw() allows anyone to force premature fund sweep

Author Revealed upon completion

Root + Impact

Description

TreasureHunt.withdraw() is intended to be an owner-only function that sweeps the contract balance to the owner once all treasures have been claimed. However, the function lacks the onlyOwner modifier. Any external caller can invoke withdraw() once claimsCount >= MAX_TREASURES, forcing the fund sweep to execute at a time of the attacker's choosing rather than the owner's.

While funds are always sent to owner and cannot be redirected, this breaks the protocol's intended access control and allows a malicious actor to grief the owner by triggering the withdrawal at an unfavourable moment (e.g., front-running the owner's own withdrawal call, or triggering it mid-hunt if the claims count condition is somehow met early via the _treasureHash bypass).

// @> Missing onlyOwner modifier — any address can call this
function withdraw() external {
if (claimsCount < MAX_TREASURES) revert HuntNotOver();
uint256 balance = address(this).balance;
if (balance == 0) revert NoFundsToWithdraw();
(bool success, ) = owner.call{value: balance}("");
if (!success) revert WithdrawFailed();
emit Withdrawn(owner, balance);
}

Risk

Likelihood:

  • The function is public with no access restriction; any watcher of the mempool can call it the moment claimsCount reaches MAX_TREASURES

  • Combined with the _treasureHash bypass (separate Critical finding), claimsCount can be incremented rapidly, making this condition reachable immediately

Impact:

  • Funds always reach the owner, so there is no theft

  • The owner loses control over the timing of the withdrawal

  • An attacker can front-run the owner's withdrawal transaction, denying the owner the ability to choose when to sweep

Proof of Concept

// Any address can call withdraw() once the hunt ends
// 1. Hunt completes: claimsCount == MAX_TREASURES
// 2. Attacker calls treasureHunt.withdraw() before the owner
// 3. Transaction succeeds — funds sent to owner, but triggered by attacker
// 4. Owner's own pending withdraw() call reverts with NoFundsToWithdraw
vm.prank(attacker);
treasureHunt.withdraw(); // succeeds — no onlyOwner check

Recommended Mitigation

- 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!