SNARKeling Treasure Hunt
First Flight #59
Beginner FriendlyGameFiFoundry
100 EXP
View all submissions
Previous Next
Submission Details
Impact: low
Likelihood: medium

`withdraw()` lacks access control, allowing any actor to trigger owner settlement

Author Revealed upon completion

withdraw() lacks access control, allowing any actor to trigger owner settlement

Description

  • Every other administrative function in the contract (fund, pause, unpause, updateVerifier, emergencyWithdraw) is restricted to the owner. withdraw() is the only exception, containing no access control and callable by any external address once claimsCount >= MAX_TREASURES.

  • Funds always go to owner, so no direct theft is possible. However, any actor can front-run the owner's intended settlement transaction, causing the owner's tx to revert with NO_FUNDS_TO_WITHDRAW and forcing the owner to rely on third parties for execution of a semantically privileged lifecycle step.

@> function withdraw() external { // @> no access control — any caller can invoke
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);
}

Risk

Likelihood:

  • Any external address can call withdraw() once claimsCount >= MAX_TREASURES, meaning the condition for exploitation is met at the natural end of every hunt cycle without any special precondition.

  • A griefer monitoring the mempool can front-run the owner's settlement transaction on every withdrawal attempt, consistently forcing the owner's tx to revert with NO_FUNDS_TO_WITHDRAW.

Impact:

  • Inconsistent access control design, as withdraw() is the only admin function not restricted to the owner, undermining the contract's intended privileged lifecycle model.

  • Owner settlement can be griefed into a revert, forcing the owner to rely on third parties for execution of a semantically privileged lifecycle step.

Proof of Concept

function test_withdrawPermissionless() public {
// Manually set claimsCount = 10 via storage manipulation (slot 2)
vm.store(address(hunt), bytes32(uint256(2)), bytes32(uint256(10)));
​
assertEq(hunt.claimsCount(), 10, "storage slot sanity check");
​
uint256 ownerBalanceBefore = owner.balance;
​
// Attacker - griefer (no stake, no benefit) triggers withdrawal
vm.prank(attacker);
hunt.withdraw();
​
// Funds went to owner nonetheless, attacker gets nothing
assertGt(owner.balance, ownerBalanceBefore);
assertEq(address(hunt).balance, 0);
​
// Owner's own attempt though now fails — griefed into revert
vm.prank(owner);
vm.expectRevert("NO_FUNDS_TO_WITHDRAW");
hunt.withdraw();
}

Recommended Mitigation

Add an onlyOwner check, consistent with every other admin function in the contract, or use the existing onlyOwner modifier:

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

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!