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

`withdraw()` is missing the `onlyOwner` guard, letting anyone force the post-hunt payout to the owner

copperbramble

Scope: contracts/src/TreasureHunt.sol

Root + Impact

Description

The post-hunt payout function is declared without any access control:

function withdraw() external { // @> no onlyOwner modifier
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);
}

The funds still flow to owner, so this is not a direct theft. But
the documented flow in the README/comments describes a
"owner withdraws after the hunt is over" step — which implies the
owner should be able to time/sequence the withdrawal. Without
onlyOwner, any third party can force-trigger the transfer the moment
claimsCount >= MAX_TREASURES becomes true, interleaving with other
post-hunt operations the owner may have planned (fresh funding,
pausing, deploying another hunt, etc.).

Risk

Likelihood: HIGH — trivially callable by anyone after the hunt
ends.

Impact: LOW — no ETH theft is possible (recipient is hard-coded
to owner), but the flow is not what the README describes.

Proof of Concept

// prerequisites: claimsCount == MAX_TREASURES, balance > 0
vm.prank(address(0xBAD)); // any EOA that is not owner
hunt.withdraw(); // succeeds; funds go to owner regardless of caller

Recommended Mitigation

Either add the onlyOwner modifier, or document explicitly that
post-hunt withdrawal is intended to be callable by anyone:

- function withdraw() external {
+ function withdraw() external onlyOwner {
require(claimsCount >= MAX_TREASURES, "HUNT_NOT_OVER");

(The onlyOwner modifier is already defined on line 53.)

Disclosure

This finding was identified and written up with the assistance of an
autonomous AI auditor (Anthropic Claude).

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!