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

[L-04] `withdraw()` is callable by anyone after hunt completion

adcdiii

Root + Impact

Description

The normal behavior is that admin fund-sweep operations are restricted to the owner.

The issue is that withdraw() has no ownership restriction. Any external account can trigger the final sweep once claimsCount >= MAX_TREASURES, forcing owner-side fund movement timing.

// contracts/src/TreasureHunt.sol
function withdraw() external { // @> missing onlyOwner check
require(claimsCount >= MAX_TREASURES, "HUNT_NOT_OVER");
uint256 balance = address(this).balance;
(bool sent, ) = owner.call{value: balance}("");
require(sent, "ETH_TRANSFER_FAILED");
}

Risk

Likelihood:

  • Any account can call withdraw() directly when the claim count condition is met.

  • Existing test confirms non-owner call succeeds.

Impact:

  • No direct fund theft occurs because withdrawn ETH is sent to owner.

  • Unauthorized actors can still force premature treasury movement and disrupt operational timing assumptions.

Severity rationale:

  • Classified as Low because the unauthorized caller cannot redirect funds, only trigger timing of a legitimate owner payout.

Proof of Concept

Standalone reproduction:

  1. Reach claimsCount == MAX_TREASURES.

  2. Ensure contract still holds ETH.

  3. Call withdraw() from a non-owner address.

  4. Observe call succeeds instead of reverting for unauthorized access.

function test_B1_WithdrawByNonOwner() public {
for (uint256 i = 0; i < 10; i++) {
address payable r = payable(address(uint160(0x6000 + i)));
vm.prank(PARTICIPANT);
hunt.claim(DUMMY_PROOF, HASH_1, r);
}
​
vm.prank(OWNER);
hunt.fund{value: 5 ether}();
​
vm.prank(ATTACKER);
hunt.withdraw(); // non-owner succeeds
}

Recommended Mitigation

Restrict the function withdraw()to only the owner

- function withdraw() external {
+ function withdraw() external onlyOwner {
require(claimsCount >= MAX_TREASURES, "HUNT_NOT_OVER");
// ...
}
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!