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

M-01: withdraw() lacks access control allowing anyone to force the post-hunt payout

altshar

Root + Impact

Description

  • withdraw() is the function the owner uses to recover leftover ETH after all treasures have been claimed. Every other privileged function in the contract is gated on msg.sender == owner, but withdraw() has no such check.

// @> no onlyOwner modifier — callable by any address
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 calls
(bool sent, ) = owner.call{value: balance}("");
require(sent, "ETH_TRANSFER_FAILED");
emit Withdrawn(balance, address(this).balance);
}

Risk

Likelihood:

  • The condition claimsCount >= MAX_TREASURES is publicly observable; any watcher can call withdraw() the moment it becomes true.

  • No funds are at risk of theft (they always go to owner), so the cost to trigger this is only gas.

Impact:

  • The owner cannot delay the withdrawal to resolve disputes or audit the final state a third party can force it immediately.

  • The Withdrawn event does not record the caller, making on-chain attribution misleading for accounting tools.

Proof of Concept

Once the claimsCount >= MAX_TREASURES condition is met, any address — including an automated bot watching the mempool can call withdraw() and trigger the payout immediately. The funds always flow to owner, so there is no direct theft, but the owner loses the ability to defer the withdrawal for any operational reason (e.g., dispute resolution, final accounting).

// After all 10 treasures are claimed:
vm.prank(attacker); // any address
hunt.withdraw(); // succeeds — 100 ETH sent to owner without owner's consent

Recommended Mitigation

Adding the onlyOwner modifier is a one-line change that brings withdraw() in line with every other privileged function in the contract. This ensures the owner retains sole discretion over when the final payout is executed, consistent with the stated design intent.

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