Last Man Standing

First Flight #45
Beginner FriendlyFoundrySolidity
100 EXP
View results
Submission Details
Impact: high
Likelihood: high
Invalid

No Access Control on withdrawPlatformFees()

Root + Impact

Description

  • The platform is expected to collect a fee from each claimThrone() action, which can later be withdrawn by the game operator to generate revenue.

  • However, the withdrawPlatformFees() function has no access control, allowing any external user to withdraw all accumulated platform fees, regardless of their role or ownership status.

// Root cause in the codebase with @> marks to highlight the relevant section
// Root cause in the codebase with @> marks to highlight the relevant section
function withdrawPlatformFees() external {
@> uint256 fees = platformFees;
@> platformFees = 0;
@> (bool success, ) = payable(msg.sender).call{value: fees}("");
require(success, "Withdraw failed");
}

Risk

Likelihood:

  • This occurs every time any user calls withdrawPlatformFees() after platform fees have been accumulated through throne claims.

  • There is no ownership check, so it will happen even if the caller is not the game deployer or owner.

Impact:

  • All platform revenue can be stolen by a malicious actor at any time.

  • The operator loses economic incentives, which could halt or discourage continued development and operation of the protocol.

Proof of Concept

This test demonstrates how any external user can withdraw all platform fees without being the contract owner.

  • First, player1 calls claimThrone and pays the required claimFee, which is partially stored as platformFees.

  • Then, player2—a malicious player who is not the owner or deployer—calls withdrawPlatformFees().

  • The balance of player2 is checked before and after the call, showing that they successfully received the platform’s funds.

  • This proves that ownership checks are missing, and anyone can steal fees from the game.

function testWithdrawPlatformFees_ByMaliciousUser() public {
// player1 becomes king and pays the claim fee
vm.prank(player1);
game.claimThrone{value: INITIAL_CLAIM_FEE}();
// malicious player2 steals platform fees
uint256 before = player2.balance;
vm.prank(player2);
game.withdrawPlatformFees();
uint256 afterBalance = player2.balance;
// assert that player2 received the platform fees
assertGt(afterBalance, before);
}

Recommended Mitigation

To fix this issue, the withdrawPlatformFees() function should only be callable by the contract owner.

  • The suggested patch adds a check:
    require(msg.sender == owner, "Only owner can withdraw platform fees");

  • This ensures only the trusted deployer or operator can access the accumulated fees.

  • Using OpenZeppelin’s Ownable contract, you can simplify this logic with the onlyOwner modifier for improved readability and standardization.

- function withdrawPlatformFees() external {
- uint256 fees = platformFees;
- platformFees = 0;
- (bool success, ) = payable(msg.sender).call{value: fees}("");
- require(success, "Withdraw failed");
- }
+ function withdrawPlatformFees() external {
+ require(msg.sender == owner, "Only owner can withdraw platform fees");
+ uint256 fees = platformFees;
+ platformFees = 0;
+ (bool success, ) = payable(msg.sender).call{value: fees}("");
+ require(success, "Withdraw failed");
+ }
Updates

Lead Judging Commences

inallhonesty Lead Judge
about 2 months ago

Appeal created

inallhonesty Lead Judge about 1 month ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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