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

`withdraw()` is publicly callable despite being documented as an owner withdrawal flow

Author Revealed upon completion

`withdraw()` is publicly callable despite being documented as an owner withdrawal flow

Description

  • The README and NatSpec describe `withdraw()` as the owner's post-hunt withdrawal path for leftover funds.

  • The implementation omits any ownership check, so any external account can trigger the withdrawal once `claimsCount >= MAX_TREASURES`. Although the funds still go to `owner`, the access-control boundary described by the project is not enforced on-chain.

/// @notice Allow the owner to withdraw unclaimed funds after the hunt is over.
function withdraw() external {
require(claimsCount >= MAX_TREASURES, "HUNT_NOT_OVER");
​
uint256 balance = address(this).balance;
require(balance > 0, "NO_FUNDS_TO_WITHDRAW");
// @> anyone can trigger the owner's withdrawal flow
(bool sent,) = owner.call{value: balance}("");
require(sent, "ETH_TRANSFER_FAILED");
}
​

Risk

Likelihood: MEDIUM

  • Any arbitrary account can call `withdraw()` as soon as the treasure-count condition is satisfied and the contract later receives additional ETH.

  • The PoC demonstrates the behavior with no privileged access by letting an attacker trigger the final withdrawal after the owner replenishes the contract.

Impact: MEDIUM

  • The contract does not enforce the access-control model communicated to users and integrators.

  • Unauthorized callers can force protocol settlement timing and trigger owner-only operational flows unexpectedly.

Proof of Concept

Paste this test function to test file and run it

function testPoCLow_AnyoneCanTriggerWithdrawFlow() public {
(bytes memory proof, bytes32 treasureHash, address payable recipient) = _loadFixture();
​
_claimTenTimes(proof, treasureHash, recipient);
​
uint256 ownerBalanceBeforeFund = owner.balance;
vm.prank(owner);
hunt.fund{value: 1 ether}();
assertEq(owner.balance, ownerBalanceBeforeFund - 1 ether);
​
uint256 ownerBalanceBeforeWithdraw = owner.balance;
​
vm.prank(attacker);
hunt.withdraw();
​
assertEq(address(hunt).balance, 0);
assertEq(owner.balance, ownerBalanceBeforeWithdraw + 1 ether);
}
​

Recommended Mitigation

Use the modifier onlyOwner that has been defined in this function

+ 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!