Last Man Standing

First Flight #45
Beginner FriendlyFoundrySolidity
100 EXP
View results
Submission Details
Severity: medium
Valid

The `claimThrone` function miscalculates the pot amount by not accounting for the previous king's payout, violating protocol expectations.

Description

  • The claimThrone function fails to consider the "previous king reward" when calculating the accumulated pot amount.

  • When computing amountToPot, the existence of previousKingPayout is completely ignored. This flawed logic causes amountToPot to be larger than it should be.

Risk

Impact:

  • After normal pot accumulation, three parties are expected to withdraw funds: the admin (platform fees), the previous king (small reward), and the current winning king (pot prize).

  • Due to the miscalculation of amountToPot that ignores the previous king's payout, the accounting becomes inconsistent.

  • This discrepancy will inevitably cause the last withdrawer to fail, as the recorded balances no longer reflect the actual available funds.

Proof of Concept

  1. This vulnerability assumes the claimThrone function has already fixed the following two issues:

    1. Correct initial check: require(msg.sender != currentKing, "Game: You are already the king. No need to re-claim.");

    2. Proper previous king reward calculation and tracking: uint256 previousKingPayout = (sentAmount * previousKngFeePercentage) / 100;

  2. Admin deploys the contract.

  3. Player player1 pays the claim fee and calls claimThrone.

  4. Player player2 pays the claim fee and calls claimThrone.

  5. Wait for the grace period to expire (1 day).

  6. Player player1 successfully withdraws their reward.

  7. Admin deployer successfully withdraws platform fees.

  8. Player player2 attempts to withdraw pot winnings but fails with: Game: Failed to withdraw winnings.

function test_Front_Running_Withdrawals() public {
uint256 cacheClaimFee = 0;
cacheClaimFee = game.claimFee();
vm.prank(player1);
game.claimThrone{value: cacheClaimFee}();
cacheClaimFee = game.claimFee();
vm.prank(player2);
game.claimThrone{value: cacheClaimFee}();
vm.warp(block.timestamp + GRACE_PERIOD + 1);
address playerAnyone = makeAddr("playerAnyone");
vm.prank(playerAnyone);
game.declareWinner();
vm.prank(deployer);
game.withdrawPlatformFees();
vm.prank(player1);
game.withdrawWinnings();
vm.prank(player2);
game.withdrawWinnings();
}

Recommended Mitigation

  • Simply adjust the amountToPot calculation to account for previousKingPayout as shown below:

function claimThrone() external payable gameNotEnded nonReentrant {
...
// Remaining amount goes to the pot
- amountToPot = sentAmount - currentPlatformFee;
+ if (previousKingPayout > 0 && currentKing != address(0)) {
+ amountToPot = sentAmount - currentPlatformFee - previousKingPayout;
+ }
+ else {
+ // If the previous king is the initial address(0), then set the previousKingPayout to the prize pool by default.
+ amountToPot = sentAmount - currentPlatformFee;
+ }
pot = pot + amountToPot;
// Update game state
currentKing = msg.sender;
lastClaimTime = block.timestamp;
playerClaimCount[msg.sender] = playerClaimCount[msg.sender] + 1;
totalClaims = totalClaims + 1;
...
}
Updates

Appeal created

inallhonesty Lead Judge 3 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Missing Previous King Payout Functionality

Support

FAQs

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