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

previousKingPayout is never initiialised in Game.sol

hunterspartan5

Root + Impact

Description

  • The previousKingPayout is never initialised, it is set to zero only. The mechanism where the current player who claims the throne and provides a small percentage of the claimFee to the previous king is compromised. There is no such code that does this operation. And also amountToPot is also initialised incorrectly. It should be also deduct the amount from sentAmount that is to be paid to the previousKing ie: payToPreviousKing (below in the code).

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 = pendingWinnings[currentKing]-pot;
// unint256 payToPreviousKing = (SOME_PERCENTAGE * claimFee) / 100;
// previousKingPayout = previousKingPayout + payToPreviousKing;
// pendingWinnings[currentKing] = previousKingPayout;
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-payToPreviousKing;
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;

Risk

Likelihood:

  • The likelihood is very high as in 100%, as the code is missing the following operation.

Impact:

  • The previousKingPayout is always zero, and the current player who claims the throne should provide a portion of its claimFee to the previous king that particular code is also not available. This will cause loss of money for the players who where kings at a point of time in the game.

Proof of Concept

​

Recommended Mitigation

Refactor the logic. Provide some portion of the claimFee to the previous king, currently that whole logic in the code is missing.

- uint256 previousKingPayout = 0;
+ uint256 previousKingPayout = pendingWinnings[currentKing]-pot;
+ unint256 payToPreviousKing = (SOME_PERCENTAGE * claimFee) / 100;
+ previousKingPayout = previousKingPayout + payToPreviousKing;
+ pendingWinnings[currentKing] = previousKingPayout;
​
- amountToPot = sentAmount - currentPlatformFee;
+ amountToPot = sentAmount-currentPlatformFee-payToPreviousKing;
Updates

Appeal created

inallhonesty Lead Judge 4 months 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.

Give us feedback!