Root + Impact
Description
The claimThrone() function contains a critical bug where the previous king receives no compensation when a new player claims the throne. The variable previousKingPayout is hardcoded to 0, which means the previous king never receives any ETH when they are dethroned. This breaks the core economic incentive mechanism of the game.
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;
}
Risk
Likelihood:
This vulnerability breaks the fundamental game economics
Previous kings lose their entire investment with no compensation
Players have no incentive to participate as they will lose 100% of their claim fee
The game becomes economically unviable and unfair to participants
Impact:
This issue occurs 100% of the time when a throne is claimed
Proof of Concept
The function claimThrone is faulty, so the POC is done via calculating what the expected output of the code is suppose to be instead of calling the function as would heve been done in a test.
pragma solidity ^0.8.20;
import {Test, console2} from "forge-std/Test.sol";
import {Game} from "../src/Game.sol";
contract PreviousKingNoPayoutTest is Test {
Game public game;
address public deployer;
address public player1;
address public player2;
uint256 public constant INITIAL_CLAIM_FEE = 0.1 ether;
uint256 public constant GRACE_PERIOD = 1 days;
uint256 public constant FEE_INCREASE_PERCENTAGE = 10;
uint256 public constant PLATFORM_FEE_PERCENTAGE = 5;
function setUp() public {
deployer = makeAddr("deployer");
player1 = makeAddr("player1");
player2 = makeAddr("player2");
vm.deal(deployer, 10 ether);
vm.deal(player1, 10 ether);
vm.deal(player2, 10 ether);
vm.startPrank(deployer);
game = new Game(
INITIAL_CLAIM_FEE,
GRACE_PERIOD,
FEE_INCREASE_PERCENTAGE,
PLATFORM_FEE_PERCENTAGE
);
vm.stopPrank();
}
function testPreviousKingReceivesNoPayout() public {
uint256 player1InitialBalance = player1.balance;
uint256 player2InitialBalance = player2.balance;
uint256 firstClaimFee = INITIAL_CLAIM_FEE;
uint256 secondClaimFee = (firstClaimFee * 110) / 100;
uint256 expectedPlatformFee = (secondClaimFee * PLATFORM_FEE_PERCENTAGE) / 100;
uint256 expectedPreviousKingPayout = 0;
uint256 expectedPotIncrease = secondClaimFee - expectedPlatformFee - expectedPreviousKingPayout;
assertEq(expectedPreviousKingPayout, 0, "Previous king payout is 0 (demonstrating the bug)");
uint256 player1Loss = firstClaimFee - expectedPreviousKingPayout;
assertEq(player1Loss, firstClaimFee, "Player1 loses entire investment");
uint256 lossPercentage = (player1Loss * 100) / firstClaimFee;
assertEq(lossPercentage, 100, "Player1 loses 100% of investment");
}
}
Recommended Mitigation
The recommended recommendation is to implement a proper payout mechanism for the previous king. Here's a suggested fix:
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 previous king payout (e.g., 70% of the claim fee)
+ if (currentKing != address(0)) {
+ previousKingPayout = (sentAmount * 70) / 100; // 70% to previous king
+ }
// Calculate platform fee on remaining amount
uint256 remainingAfterKingPayout = sentAmount - previousKingPayout;
currentPlatformFee = (remainingAfterKingPayout * platformFeePercentage) / 100;
// Remaining amount goes to the pot
amountToPot = sentAmount - previousKingPayout - currentPlatformFee;
// Send payout to previous king
+ if (previousKingPayout > 0) {
+ (bool success, ) = payable(currentKing).call{value: previousKingPayout}("");
+ require(success, "Game: Failed to send payout to previous king.");
+ }
}
