Last Man Standing
First Flight #45
Beginner FriendlyFoundrySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Valid

Missing previousKingPayout logic in claimThrone function

0xdelvine

Root + Impact

Description

The claimThrone function is intended to allow a new player to claim the throne by sending the required claim fee, updating the game state, and distributing a portion of the claim fee to the previous king as an incentive, as indicated by the comment: “If there’s a previous king, a small portion of the new claim fee is sent to them.” However, the code initializes previousKingPayout to 0 and never updates it, failing to implement the payout logic for the previous king.

// Root cause in the codebase with @> marks to highlight the relevant section
function claimThrone() external payable gameNotEnded nonReentrant {
require(msg.value >= claimFee, "Game: Insufficient ETH sent to claim the throne.");
require(msg.sender != currentKing, "Game: You are already the king. No need to re-claim.");
uint256 sentAmount = msg.value;
@> uint256 previousKingPayout = 0;
uint256 currentPlatformFee = 0;
uint256 amountToPot = 0;
​
// Calculate platform fee
currentPlatformFee = (sentAmount * platformFeePercentage) / 100;
​
// Defensive check to ensure platformFee doesn't exceed available amount after previousKingPayout
if (currentPlatformFee > (sentAmount - previousKingPayout)) {
currentPlatformFee = sentAmount - previousKingPayout;
}
platformFeesBalance = platformFeesBalance + currentPlatformFee;
​
// Remaining amount goes to the pot
@> amountToPot = sentAmount - currentPlatformFee;
// ... rest of the function
}

Risk

Likelihood:

  • Occurs every time a new player claims the throne when there is an existing currentKing, as the previousKingPayout is always set to 0 and no funds are allocated to the previous king.

  • Affects all claims after the first in each game round, since the logic for rewarding the previous king is entirely absent.

Impact:

  • Breaks the intended game mechanic of rewarding the previous king, which could discourage players from participating if they expect a payout for their time as king.

Proof of Concept

function testPreviousKingGetsNoPayout() public {
// NOTE: This POC assumes the claimThrone logic bug is fixed
// (changing require(msg.sender == currentKing) to require(msg.sender != currentKing))
​
// Alice claims throne first
vm.prank(alice);
game.claimThrone{value: 1 ether}();
// Check Alice's pending winnings before Bob claims
uint256 aliceWinningsBefore = game.pendingWinnings(alice);
// Bob claims throne from Alice (should give Alice some payout)
vm.prank(bob);
game.claimThrone{value: 1.1 ether}();
// BUG PROOF: Alice (previous king) gets no payout
uint256 aliceWinningsAfter = game.pendingWinnings(alice);
assertEq(aliceWinningsAfter, aliceWinningsBefore); // Should fail if Alice got paid
// Alice got 0 payout despite being dethroned
assertEq(aliceWinningsAfter, 0);
}

Note: This POC cannot be executed with the current contract since the bug requires 2 kings and the first one to be dethroned in order to get the payout, but the primary claimThrone logic bug prevents anyone except the current king from claiming the throne. If you correct this line in Game.sol:

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

Then the test would work perfectly and prove the missing payout bug:

  1. Alice claims first (works because Alice != address(0))

  2. Bob claims from Alice (works because Bob != Alice)

  3. Alice gets 0 payout (proves the bug - she should get some portion)

The test would pass and demonstrate that:

  • aliceWinningsBefore = 0

  • aliceWinningsAfter = 0

  • Alice received no compensation for being dethroned

  • All of Bob's 1.1 ether went to pot + platform fees instead of giving Alice her deserved cut

Recommended Mitigation

Introduce a configurable reward percentage for the dethroned king, and update the payout logic accordingly:

+ uint256 public previousKingRewardPercentage; // e.g. 20 for 20%
​
function claimThrone() external payable gameNotEnded nonReentrant {
require(msg.value >= claimFee, "Game: Insufficient ETH sent to claim the throne.");
require(msg.sender != currentKing, "Game: You are already the king. No need to re-claim.");
​
uint256 sentAmount = msg.value;
- uint256 previousKingPayout = 0;
+ uint256 previousKingPayout = (sentAmount * previousKingRewardPercentage) / 100;
​
uint256 currentPlatformFee = (sentAmount * platformFeePercentage) / 100;
​
// Prevent over-allocation
uint256 totalCut = previousKingPayout + currentPlatformFee;
require(totalCut <= sentAmount, "Game: Invalid fee configuration.");
​
- // No payout logic
+ if (currentKing != address(0)) {
+ pendingWinnings[currentKing] += previousKingPayout;
+ }
​
platformFeesBalance += currentPlatformFee;
​
uint256 amountToPot = sentAmount - totalCut;
pot += amountToPot;
​
currentKing = msg.sender;
lastClaimTime = block.timestamp;
...
}
Updates

Appeal created

inallhonesty Lead Judge about 2 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.