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

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

moumm

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