SNARKeling Treasure Hunt

First Flight #59
Beginner FriendlyGameFiFoundry
100 EXP
Submission Details
Impact: high
Likelihood: high

withdraw() Missing Access Control

Author Revealed upon completion

Root + Impact

Description

  • As the doc note the withdraw()function should be called by owner.

  • Due to the bug, any user can call the withdraw()function.


//file: TreasureHunt.sol
/// @notice Allow the owner to withdraw unclaimed funds after the hunt is over.
function withdraw() external { // Any user can call this function
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:

  • Reason 1: Due to lack of access control, any user can call withdraw()function.


Impact:

  • Impact 1: The withdraw() function lacks any access control modifier or msg.sender check, allowing any external address to trigger the withdrawal of all contract funds to the owner. While funds are sent to the correct owner address, the timing and initiation should be under owner control.

Proof of Concept

Any user can successfully call the withdraw()function when other requirements are met.

Game starts...
all treasures are discovered and rewards claimed.
A non-owner user call the `withdraw()` function.
Remaining fund is successfully sent to the owner.

Recommended Mitigation

Add access control, a predefined modifier onlyowner, to the function:

//file: TreasureHunt.sol
/// @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");

Support

FAQs

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

Give us feedback!