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

NO PAYOUT TO THE PREVIOUS KING !!

nkh000

No payout made to the previos king when a new king claims the throne using claimeThrone function

Description

According to the protocolls description, when a new king claims the throne than a small payout has to be paid to previous king. But not only previous king is not getting paid, amount to be paid to previous king is also hardcoded to be 0 (zero)

  • Normally, when next king claims the throne, previous king receives a small payout from next player's claimFee

  • In this scenario, payout for previous king is not only not calculated and is zero through out the process but also all the funds is distributed to platformFee and pot

  • MIssing transfer logic, no any low level call, transfer or send function call to the previous king's address for the payout.

  • Can be considered fradulent if players expects reward.

function claimThrone() external payable gameNotEnded nonReentrant {
require(
msg.value >= claimFee,
"Game: Insufficient ETH sent to claim the throne."
);
// @notice this line has been changed to msg.sender != currentKing (from original msg.sender == currentKing) to make the function work
require(
msg.sender != currentKing,
"Game: You are already the king. No need to re-claim."
);
​
uint256 sentAmount = msg.value;
// @ audit-high hardcoded value for previous king payout which will always be zero as it never updated through out the function
@> 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;
...
}

Risk

Likelihood:

  • Will happen everytime when some one claims the throne.

Impact:

  • If players believe they will receive a reward upon being dethroned (as is standard in similar games), the lack of such payout could be legally or ethically questionable.

Proof of Concept

Paste this test function inside Game.t.sol for testing

function testNoIncreamentInPreviousKingsBalanceAfterNextClaim() public {
vm.prank(player1);
game.claimThrone{value: 1 ether}();
uint256 player1BalanceBeforeNextClaim = address(player1).balance;
​
vm.prank(player2);
game.claimThrone{value: 1 ether}();
​
uint256 player1BalanceAfterNextClaim = address(player2).balance;
assertEq(player1BalanceBeforeNextClaim, player1BalanceAfterNextClaim);
}

Recommended Mitigation

To prevent this behaviour of the protocol, We can apply following measures to ensure previous king gets payout when next claim is claimed.

  • Add a percantage for previous king payout from the next claimFee .

  • Additionally adding a MINIMUM and MAXIMUM (in percantage) value a king can get as a payout from the protocol after the next claim.

  • Defensive checks to make sure payout does not exceed more than Max and Min values of Payout.

Simple Example of how this could work is.

- uint256 previousKingPayout = 0;
// making sure that previousKingPayoutPercantage should not be greater than for eg; 30% or it would take every fund not leaving anything for the pot
+ uint256 previousKingPayout = (sentAmount * previousKingPayoutPercentage) / 100;
​
​
// ETH transfer to previous king
+ if (previousKing != address(0) && previousKing != msg.sender) {
+ (bool success, ) = previousKing.call{value: previousKingPayout}("");
+ require(success, "Previous king payout failed");
+ }
Updates

Appeal created

inallhonesty Lead Judge about 2 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.