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

`withdraw()` is callable by any address after the hunt ends

Author Revealed upon completion

Root + Impact

Description

  • Normal behavior: Post-hunt withdrawal of leftover funds is an organizer-controlled action, or it is documented that any party may trigger settlement.

  • Problem: withdraw() has no onlyOwner modifier. Once claimsCount >= MAX_TREASURES, any caller can invoke it. ETH still goes to owner, but the caller and timing are not restricted to the owner.

// @> no access modifier — any msg.sender may call after claimsCount >= MAX_TREASURES
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:

  • After the tenth claim, any EOA or contract can call withdraw() on the next block or later.

  • Bots or third parties routinely observe contract state and call public settlement-style functions.

Impact:

  • Owner loses exclusive control over when post-hunt funds are swept, which matters for accounting, taxes, and coordination with off-chain hunt closure.

  • No direct theft to an arbitrary address here, but policy and operational expectations break.

Proof of Concept

Explanation: After ten claims, ATTACKER calls withdraw() while OWNER receives the ETH. The test proves msg.sender need not be the owner.

Supporting code — run (fixtures from circuits/scripts/build.sh):

forge test --match-test testPoC_AnyoneCanForceHuntEndAndTriggerWithdraw -vv
​

Supporting code — test:

// contracts/test/TreasureHuntPoC.t.sol — testPoC_AnyoneCanForceHuntEndAndTriggerWithdraw
function testPoC_AnyoneCanForceHuntEndAndTriggerWithdraw() public {
vm.startPrank(OWNER);
hunt.fund{value: 20 ether}();
vm.stopPrank();
​
(bytes memory proof, bytes32 treasureHash, address payable recipient) = _fixture();
vm.startPrank(PARTICIPANT);
for (uint256 i = 0; i < 10; i++) {
hunt.claim(proof, treasureHash, recipient);
}
vm.stopPrank();
​
uint256 ownerBefore = OWNER.balance;
vm.prank(ATTACKER);
hunt.withdraw();
​
assertEq(OWNER.balance, ownerBefore + 20 ether);
}
​

Recommended Mitigation

Explanation: Post-hunt withdrawal should be an owner-controlled administrative action. Restricting withdraw with onlyOwner preserves exclusive control over timing and accounting while keeping funds routed to owner as today.

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