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

withdraw() has no access control. anyone can trigger owner withdrawal.

0xkhlo

ROOT: ** **contracts/src/TreasureHunt.sol — withdraw() is missing onlyOwner modifier present on all other admin functions
IMPACT: Anyone can trigger the owner withdrawal, griefing the owner and wasting their gas

Description

  • The withdraw() function is intended to let the owner recover leftover funds after the hunt concludes. All other admin functions use onlyOwner or an equivalent require check.

// @> missing onlyOwner — compare with pause(), unpause(), fund(), emergencyWithdraw()
function withdraw() external {
require(claimsCount >= MAX_TREASURES, "HUNT_NOT_OVER");
uint256 balance = address(this).balance;
require(balance > 0, "NO_FUNDS_TO_WITHDRAW");
// @> funds go to owner regardless of who called
(bool sent, ) = owner.call{value: balance}("");
require(sent, "ETH_TRANSFER_FAILED");
emit Withdrawn(balance, address(this).balance);
}

Risk

Likelihood:

  • Any address calls withdraw() after all treasures are claimed — the condition claimsCount >= MAX_TREASURES is publicly readable so anyone knows when it becomes callable

  • A griefing actor front-runs the owner's withdrawal transaction, forcing the owner to pay gas for a transaction that never executes

Impact:

  • No financial loss funds always reach the owner

  • Owner loses control over withdrawal timing and wastes gas if front-run

Proof of Concept

Anyone can call this once claimsCount reaches MAX_TREASURES

​
ITreasureHunt(hunt).withdraw(); // succeeds, sends full balance to owner

Recommended Mitigation

- function withdraw() external {
+ function withdraw() external onlyOwner {
require(claimsCount >= MAX_TREASURES, "HUNT_NOT_OVER");
uint256 balance = address(this).balance;
require(balance > 0, "NO_FUNDS_TO_WITHDRAW");
(bool sent, ) = owner.call{value: balance}("");
require(sent, "ETH_TRANSFER_FAILED");
emit Withdrawn(balance, address(this).balance);
}
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!