Root + Impact
Description
-
The claimThrone() function lets a new player claim the throne by paying a fee. The previous king should receive compensation when overthrown.
-
The contract fails to credit the previous king with any compensation, causing them to lose funds unfairly.
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.");
uint256 sentAmount = msg.value;
uint256 previousKingPayout = 0;
uint256 currentPlatformFee = 0;
uint256 amountToPot = 0;
currentPlatformFee = (sentAmount * platformFeePercentage) / 100;
if (currentPlatformFee > (sentAmount - previousKingPayout)) {
currentPlatformFee = sentAmount - previousKingPayout;
}
platformFeesBalance = platformFeesBalance + currentPlatformFee;
amountToPot = sentAmount - currentPlatformFee;
pot = pot + amountToPot;
currentKing = msg.sender;
lastClaimTime = block.timestamp;
playerClaimCount[msg.sender] = playerClaimCount[msg.sender] + 1;
totalClaims = totalClaims + 1;
claimFee = claimFee + (claimFee * feeIncreasePercentage) / 100;
emit ThroneClaimed(
msg.sender,
sentAmount,
claimFee,
pot,
block.timestamp
);
}
Risk
Likelihood:
-
Occurs whenever a new player claims the throne, overthrowing the current king
-
Happens consistently due to missing logic for compensating the previous king on throne claim.
Impact:
-
The previous king receives no compensation after being overthrown.
-
Players lose funds they are entitled to, harming fairness and trust in the game.
Proof of Concept
Explanation:
This test simulates two players sequentially claiming the throne. Before the fix, the first player (previous king) receives no compensation after being overthrown, causing withdrawal to fail. After applying the fix, the previous king correctly receives a pending payout and can successfully withdraw their winnings. This confirms the presence of the bug and verifies that the fix resolves it.
pragma solidity ^0.8.20;
import "forge-std/Test.sol";
import "../src/Game.sol";
contract NoCompensationBugTest is Test {
Game game;
address playerA = address(0x1);
address playerB = address(0x2);
function setUp() public {
game = new Game(1 ether, 60, 10, 5);
vm.deal(playerA, 5 ether);
vm.deal(playerB, 5 ether);
}
function test_PreviousKingReceivesNothing_ShouldFailBeforeFix() public {
vm.startPrank(playerA);
game.claimThrone{value: game.claimFee()}();
vm.stopPrank();
vm.startPrank(playerB);
game.claimThrone{value: game.claimFee()}();
vm.stopPrank();
vm.startPrank(playerA);
vm.expectRevert("Game: No winnings to withdraw.");
game.withdrawWinnings();
vm.stopPrank();
}
function test_PreviousKingGetsCompensated_AfterFix() public {
vm.startPrank(playerA);
game.claimThrone{value: game.claimFee()}();
vm.stopPrank();
vm.startPrank(playerB);
game.claimThrone{value: game.claimFee()}();
vm.stopPrank();
uint256 payout = game.pendingWinnings(playerA);
assertGt(payout, 0);
vm.startPrank(playerA);
uint256 balBefore = playerA.balance;
game.withdrawWinnings();
uint256 balAfter = playerA.balance;
vm.stopPrank();
assertGt(balAfter, balBefore);
}
}
Recommended Mitigation
Explanation:
The fix adds compensation for the previous king by allocating a portion of the claim amount to their pending winnings before calculating platform fees and updating the pot. This ensures the previous king is properly rewarded when overthrown.
- // Calculate platform fee
- currentPlatformFee = (sentAmount * platformFeePercentage) / 100;
-
- // Remaining amount goes to the pot
- amountToPot = sentAmount - currentPlatformFee;
+ // Compensate previous king if exists
+ if (currentKing != address(0)) {
+ previousKingPayout = (sentAmount * 50) / 100;
+ pendingWinnings[currentKing] += previousKingPayout;
+ }
+
+ // Calculate platform fee on remaining amount
+ currentPlatformFee = (sentAmount * platformFeePercentage) / 100;
+
+ // Remaining amount after payouts and fees goes to pot
+ amountToPot = sentAmount - previousKingPayout - currentPlatformFee;
