Submission Details
Severity:
Incorrect Handling of previous King payout in claimthrone function
Root + Impact
Description
-
In claimthrone function, the player can claim the throne by sending the required claimfee
-
The issue is that in the current logic, the previous king payout is reflecting as 0 and there is no reward for the previous king
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;
@i>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;
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
);
}
Risk
Likelihood:
It occurs when the player wants to claimthrone by sending the claim fees
Impact:
It basically creates misunderstanding in terms of finance for the user that there is no previous king payout
Proof of Concept
Add the following test function in the Game.t.sol file
function testDethronedKingReceivesNoPayout() public {
// Player 1 claims the throne
vm.startPrank(player1);
game.claimThrone{value: INITIAL_CLAIM_FEE}();
vm.stopPrank();
// Record balance after player1 becomes king
uint256 balanceAfterClaim = player1.balance;
// Calculate expected next claim fee
uint256 nextClaimFee = INITIAL_CLAIM_FEE + (INITIAL_CLAIM_FEE * FEE_INCREASE_PERCENTAGE) / 100;
// Player 2 claims the throne
vm.startPrank(player2);
game.claimThrone{value: nextClaimFee}();
vm.stopPrank();
// Check if player1 got paid anything (they should not)
uint256 balanceAfterDethroned = player1.balance;
// Assert that dethroned player1's balance is unchanged (i.e., no payout)
assertEq(balanceAfterDethroned, balanceAfterClaim, "Dethroned king should not receive a payout (this is the bug).");
}
Recommended Mitigation
I have removed the previousKingpayout variable along with currentplatform fee and amount to pot variable and then refactor logic to reward dethorned king
+ uint256 public dethronedKingRewardPercentage;
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.");
- 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;
// Store current king before updating
+ address payable previousKing = payable(currentKing);
// Calculate shares
+ uint256 dethronedReward = (sentAmount * dethronedKingRewardPercentage) / 100;
+ uint256 platformCut = (sentAmount * platformFeePercentage) / 100;
+ uint256 potShare = sentAmount - platformCut - dethronedReward;
// Update state
currentKing = msg.sender;
lastClaimTime = block.timestamp;
+ playerClaimCount[msg.sender] += 1;
+ totalClaims += 1;
+ platformFeesBalance += platformCut;
+ pot += potShare;
// Increase claim fee
claimFee = claimFee + (claimFee * feeIncreasePercentage) / 100;
// Send dethroned king their reward (last, to avoid reentrancy)
+ if (previousKing != address(0)) {
+ (bool success, ) = previousKing.call{value: dethronedReward}("");
+ require(success, "Game: Failed to send reward to dethroned king");
+ }
emit ThroneClaimed(msg.sender, sentAmount, claimFee, pot, block.timestamp);
}
Updates
Appeal created
inallhonesty about 1 month ago
Submission Judgement Published Assigned finding tags:
Missing Previous King Payout Functionality
Rewards breakdown
nSLOC
181This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.