Root + Impact
Description
-
According to the comment of claimThrone(), previous king will receive next king's small portion of the claim fee
-
However, although previousKingPayout is declared and initialized to 0, it is never set and transfered to previous king
Note that below code has fixed the incorrect access control require(msg.sender == currentKing, "Game: You are already the king. No need to re-claim.");, in order to make the core basic functionality work
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;
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: High
Impact: High
-
Financial Loss for Players: Dethroned kings incurs a cost when claiming the throne but receive no compensation, resulting in an unfair financial disadvantage.
-
Credibility Degradation: The mismatch between documented game mechanics and actual implementation undermines user trust and damages the project’s reputation.
-
Broken Incentive Structure: Without rewards for holding the throne, players have no motivation to participate early or repeatedly, leading to poor engagement and a shift toward last-minute sniping strategies
Proof of Concept
Add the following test, then run the command: forge test -vv --match-test testNoRecevivePayoutFromNextPlayer
function testNoRecevivePayoutFromNextPlayer() public {
console2.log("player1 claims the throne");
uint256 claimFee = game.claimFee();
vm.prank(player1);
game.claimThrone{value: claimFee}();
console2.log("player1 balance: ", address(this).balance);
console2.log("player2 claims the throne");
claimFee = game.claimFee();
vm.prank(player2);
game.claimThrone{value: game.claimFee()}();
console2.log("player1 balance: ", address(this).balance);
console2.log("player1 doesn't receive the payout from player2");
}
PoC Results:
forge test -vv --match-test testNoRecevivePayoutFromNextPlayer
[⠊] Compiling...
[⠑] Compiling 1 files with Solc 0.8.29
[⠘] Solc 0.8.29 finished in 668.41ms
Compiler run successful!
Ran 1 test for test/Game.t.sol:GameTest
[PASS] testNoRecevivePayoutFromNextPlayer() (gas: 200452)
Logs:
player1 claims the throne
player1 balance: 79228162514264337593543950335
player2 claims the throne
player1 balance: 79228162514154337593543950335
player1 doesn't receive the payout from player2
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 7.37ms (1.50ms CPU time)
Ran 1 test suite in 324.91ms (7.37ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Recommended Mitigation
-
Add a state variable previousKingPayoutPercentage and initialize it inside constrcutor, ensure that _previousKingRewardPercentage + _platformFeePercentage <= 100
-
Add the function updatePreviousKingPayout(uint256 _newPreviousKingPayout) if needed
-
insert the logic of payout for previsou king inisde claimThrone()
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;
+ address previousKing = currentKing;
// Calculate platform fee
currentPlatformFee = (sentAmount * platformFeePercentage) / 100;
+ previousKingPayout = (sentAmount * previousKingPayoutPercentage) / 100;
// Defensive check to ensure platformFee doesn't exceed available amount after previousKingPayout
if (currentPlatformFee > (sentAmount - previousKingPayout)) {
currentPlatformFee = sentAmount - previousKingPayout;
}
platformFeesBalance = platformFeesBalance + currentPlatformFee;
+ (bool success, ) = payable(previousKing).call{value: previousKingPayout}("");
+ require(success, "Game: Failed to pay for previous king.");
// Remaining amount goes to the pot
+ amountToPot = sentAmount - currentPlatformFee - previousKingPayout;
- 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;
// Increase the claim fee for the next player
claimFee = claimFee + (claimFee * feeIncreasePercentage) / 100;
emit ThroneClaimed(
msg.sender,
sentAmount,
claimFee,
pot,
block.timestamp
);
}
