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

Previous King Does Not Receive Promised Payout, Resulting in a Loss of User Funds

wojack0x0

Description

The game's official documentation (README.md) states that a key incentive for players is that the "King... Receives a small payout from the next player's claimFee". This mechanic is intended to reward players for participating, even if they are eventually overthrown.

However, the claimThrone() function completely omits the logic required to calculate and distribute this payout. The function does not save the address of the currentKing before overwriting it with the new king. Consequently, there is no mechanism to credit the previous king with their share of the fee, and these funds are instead allocated entirely to the pot and platform fees. This constitutes a direct loss of entitled funds for every player who becomes king and is later dethroned.

// src/Game.sol:230-257
function claimThrone() external payable gameNotEnded nonReentrant {
// ... checks ...
// @> `previousKingPayout` is never calculated or used. The previous king's address is lost.
// ...
currentKing = msg.sender;
// ...
// @> No logic exists to transfer a payout to the previous king.
pot = pot + amountToPot;
// ...
}

Risk

Likelihood: High

  • This loss of funds occurs on every successful throne claim after the first one, in every game round. It is a guaranteed outcome of the current implementation.

Impact: Medium

  • Direct Loss of User Funds: Players are denied a promised and documented financial reward, breaking a core economic incentive of the game.

  • Broken Game Mechanic: The incentive structure described to players is not correctly implemented, leading to a misleading and less rewarding game experience.

  • Erosion of Trust: Users who read the documentation will expect this payout and may lose trust in the protocol when they realize it is not being honored.

Proof of Concept

The following test proves that the contract's internal logic fails to account for the previous king's payout. It manually sets the game state and then verifies that the pendingWinnings storage slot for the previous king remains zero after a simulated takeover, confirming the flaw.

Test File: test/MissingPayout.t.sol

// test/MissingPayout.t.sol
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;
​
import {Test} from "forge-std/Test.sol";
import {Game} from "../../src/Game.sol";
​
contract MissingPayoutTest is Test {
Game public game;
address public deployer;
address public player1;
​
function setUp() public {
deployer = makeAddr("deployer");
player1 = makeAddr("player1");
vm.deal(deployer, 1 ether);
vm.deal(player1, 1 ether);
vm.startPrank(deployer);
// Using a 10% platform fee for the test. We will assume a 10% king payout.
game = new Game(0.1 ether, 1 days, 10, 10);
vm.stopPrank();
}
​
function test_PoC_PreviousKingReceivesNoPayout() public {
// --- Setup: Manually place the game in a state where player1 is the king ---
bytes32 kingSlot = bytes32(uint256(1)); // From `forge inspect storage-layout`
vm.store(address(game), kingSlot, bytes32(uint256(uint160(player1))));
assertEq(game.currentKing(), player1);
uint256 player1WinningsBefore = game.pendingWinnings(player1);
assertEq(player1WinningsBefore, 0);
​
// --- Simulate the flawed logic's outcome ---
uint256 sentAmount = 0.11 ether; // A hypothetical amount from a new king
uint256 expectedKingPayout = (sentAmount * 10) / 100; // Assuming a 10% payout from docs
// --- Assertion ---
// The core issue: The `pendingWinnings` mapping for player1 is never updated by the contract.
// We verify this by directly reading the storage slot for `pendingWinnings[player1]`.
bytes32 winningsSlot = keccak256(abi.encode(player1, uint256(11))); // Slot 11 from inspect
bytes32 storedWinnings = vm.load(address(game), winningsSlot);
assertEq(uint256(storedWinnings), 0, "VULNERABILITY: Player 1's winnings remain 0 in storage after being dethroned.");
assertNotEq(expectedKingPayout, 0, "The promised payout should have been a non-zero amount.");
}
}

Successful Test Output:

$ forge test --match-path test/MissingPayout.t.sol
[PASS] test_PoC_PreviousKingReceivesNoPayout() (gas: 22365)
Suite result: ok. 1 passed; 0 failed; 0 skipped

The successful test confirms that the pendingWinnings for the previous king remains zero, as the current contract implementation contains no logic to handle the documented payout, thus validating the vulnerability.

Recommended Mitigation

The payout logic needs to be implemented in claimThrone(). This involves storing the previous king's address, calculating their share, adding it to their pendingWinnings, and subtracting it from the amount that goes to the pot. A new state variable for the king's payout percentage should also be introduced.

// src/Game.sol
​
// --- State Variables ---
// ...
uint256 public platformFeePercentage;
+ uint256 public kingPayoutPercentage;
uint256 public initialGracePeriod;
// ...
​
// In constructor
constructor(
//...
uint256 _platformFeePercentage,
+ uint256 _kingPayoutPercentage // e.g., 10 for 10%
) Ownable(msg.sender) {
// ...
- require(_platformFeePercentage <= 100, "Game: Platform fee percentage must be 0-100.");
+ require(_platformFeePercentage + _kingPayoutPercentage <= 100, "Game: Total fees cannot exceed 100%.");
platformFeePercentage = _platformFeePercentage;
+ kingPayoutPercentage = _kingPayoutPercentage;
// ...
}
​
// In claimThrone()
function claimThrone() external payable gameNotEnded nonReentrant {
// ... (checks remain the same) ...
uint256 sentAmount = msg.value;
- uint256 previousKingPayout = 0;
uint256 amountToPot = sentAmount;
+ address previousKing = currentKing;
​
// Calculate and deduct platform fee
uint256 currentPlatformFee = (sentAmount * platformFeePercentage) / 100;
platformFeesBalance = platformFeesBalance + currentPlatformFee;
+ amountToPot = amountToPot - currentPlatformFee;
​
+ // Calculate and assign previous king's payout if one exists
+ if (previousKing != address(0)) {
+ uint256 kingPayout = (sentAmount * kingPayoutPercentage) / 100;
+ // Defensive check to ensure payout doesn't exceed available funds
+ if (kingPayout > amountToPot) {
+ kingPayout = amountToPot;
+ }
+ pendingWinnings[previousKing] = pendingWinnings[previousKing] + kingPayout;
+ amountToPot = amountToPot - kingPayout;
+ }
​
- // ... (old fee logic can be removed) ...
​
pot = pot + amountToPot;
currentKing = msg.sender;
// ...
}
Updates

Appeal created

inallhonesty Lead Judge 10 days 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.