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

Lack of Access Control, anyone can trigger the withdraw function

Author Revealed upon completion

Lack of Access Control, anyone can trigger the withdraw function

Description

  • The withdraw() function is intended to be an administrative action that allows the contract owner to sweep the treasury balance once the treasure hunt has concluded (when claimsCount meets or exceeds MAX_TREASURES).

  • The function is missing an access control modifier (such as onlyOwner), allowing any external account to trigger the transfer of the contract's entire Ether balance to the owner's address.

​
// Root cause in the codebase with @> marks to highlight the relevant section
​
​
// @> function withdraw() external {
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:

  • The function visibility is set to external and contains no caller validation, making it accessible to any user on the network

  • Blockchain monitoring bots (griefers) constantly scan for unprotected functions to execute them as soon as the state-based requirements (claimsCount) are satisfied.

Impact:

  • An unauthorized party can force a withdrawal at an inconvenient time for the owner, potentially causing issues with tax accounting, fund management, or cold-storage readiness.

  • If the owner is a smart contract (like a multisig) that is temporarily unable to receive ETH due to a configuration error, an attacker can intentionally trigger the withdrawal to "lock" the function in a failed state or force the owner to deal with the failure prematurely.

Proof of Concept

The PoC demonstrates that the withdraw() function relies entirely on system state rather than caller identity.

  1. State Bypassing: The test uses low-level storage manipulation (vm.store) to force the contract into a "Post-Game" state. This proves that the vulnerability is not tied to the game logic itself, but exists as a standalone flaw once the game's conditions are met.

  2. Access Verification: By executing the function from a randomly generated address (the attacker), the PoC confirms that the Ethereum Virtual Machine (EVM) finds no restrictions on who can initiate the transaction.

  3. Resulting Action: The PoC concludes by verifying that the contract balance was successfully moved. Even though the funds were sent to the owner, the fact that an unauthorized third party could dictate the timing and execution of this high-value transfer is the core of the proof.

// Run with: forge test --match-test test_PoC_LackOfAccessControl
function test_PoC_LackOfAccessControl() public {
// Setup: Contract has funds
vm.deal(address(hunt), 10 ether);
uint256 max = hunt.MAX_TREASURES();
​
// Simulate hunt completion via storage manipulation
for (uint256 i = 0; i < 10; i++) {
vm.store(address(hunt), bytes32(i), bytes32(max));
if (hunt.claimsCount() == max) break;
}
​
// Unauthorized user triggers withdrawal
vm.prank(address(0xBAD));
hunt.withdraw();
​
// Verification: Funds were moved despite unauthorized caller
assertEq(address(hunt).balance, 0);
}

Recommended Mitigation

By adding the onlyOwner modifier, the contract introduces a mandatory check at the very beginning of the function execution. This check compares the msg.sender (the person calling the function) against the stored owner address.

​
-function withdraw() external {
+function withdraw() external onlyOwner{
​

Support

FAQs

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

Give us feedback!