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

Missing access control on withdraw function allows permissionless execution

Author Revealed upon completion

Root + Impact

Description

  • Normal Behavior: Administrative functions that handle the sweeping of remaining or unclaimed funds are strictly designed to be executed by the contract owner. This allows the protocol administrators to manage their treasury, calculate taxes, or move funds to a cold wallet on their own specific schedule.

  • Specific Issue: The withdraw function lacks the onlyOwner modifier. The function only checks if the hunt is over (claimsCount >= MAX_TREASURES). Once this condition is met, any external user, bot, or attacker can call the function. While the funds are hardcoded to be sent to the owner address (preventing theft), this permissionless execution removes the owner's control over the exact timing of the withdrawal, which can disrupt off-chain accounting systems or trigger unwanted tax events for the protocol.

// ----- admin functions -----
​
/// @notice Allow the owner to withdraw unclaimed funds after the hunt is over.
// @> Root cause: Missing onlyOwner modifier allows anyone to trigger the sweep.
function withdraw() external {
require(claimsCount >= MAX_TREASURES, "HUNT_NOT_OVER");
​
uint256 balance = address(this).balance;

Risk

Likelihood:

  • Any user or MEV bot can execute this function immediately after the final treasure is claimed.

Impact:

  • No direct loss of funds, as the ETH is securely routed to the owner address.

  • Protocol griefing through forced operational timing. The owner loses control over when the transaction occurs.

Proof of Concept

The following Foundry test demonstrates how an unauthorized user can force the withdrawal:

  1. The test simulates a state where all 10 treasures have been successfully claimed.

  2. A random user (attackerAddress) calls the withdraw() function.

  3. Because there is no msg.sender == owner check, the transaction succeeds.

  4. The remaining contract balance is forcefully pushed to the owner without their consent.

function testAnyUserCanForceWithdrawal() public {
// 1. Simulate the end of the game
vm.store(address(treasureHunt), bytes32(uint256(1)), bytes32(uint256(10))); // Set claimsCount to 10
// 2. Prank as an unauthorized user
address randomUser = address(0x1337);
vm.prank(randomUser);
// 3. Execute the admin withdrawal function
treasureHunt.withdraw(); // @> Succeeds without reverting
// 4. Verify funds were swept
assertEq(address(treasureHunt).balance, 0);
}

Recommended Mitigation

Explanation of the fix: Implementing the existing onlyOwner modifier ensures that the EVM checks msg.sender == owner before executing the logic. This restores full control to the protocol administrators regarding when the remaining treasury is swept.

/// @notice Allow the owner to withdraw unclaimed funds after the hunt is over.
- function withdraw() external {
+ function withdraw() external onlyOwner {
require(claimsCount >= MAX_TREASURES, "HUNT_NOT_OVER");
​
uint256 balance = address(this).balance;

Support

FAQs

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

Give us feedback!