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

Incorrect Require Condition in Game.sol::claimThrone

legacy

Root + Impact

Description

The Game.sol:: claimThrone function**** uses**** require(msg.sender == currentKing) . This check ensures that the caller is the current king instead of checking that the caller is not the current king. As written, no new player can ever claim the throne (and in fact the very first claim fails because currentKing is initially 0). This breaks basic game functionality and renders the whole game unplayable.

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;
​
// 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:

  • The likelihood is very high as it affects the protocol from the very first instance a player tries to interact with the game.


Impact:

  • Impact 1 : This logic bug makes the game unplayable by users and therefore renders the whole protocol obsolete.

  • Impact 2: Due to this issue, there would be no protocol fee to be claimed and the platform generates no revenue.

Proof of Concept

The below test case shows the issue:
​
function testGameBroken_NoOneCanClaimThrone() public {
// Initial state: currentKing is address(0)
assertEq(game.currentKing(), address(0));
vm.startPrank(player1);
// This will REVERT because msg.sender (player1) != currentKing (address(0))
// But the require statement checks msg.sender == currentKing!
vm.expectRevert("Game: You are already the king. No need to re-claim.");
game.claimThrone{value: INITIAL_CLAIM_FEE}();
vm.stopPrank();
// Verify game state hasn't changed
assertEq(game.currentKing(), address(0));
assertEq(game.pot(), 0);
assertEq(game.totalClaims(), 0);
}

From the above test case, it is evident how the test passes and proves how the game is rendered unplayable due to the fact that the require check is not correctly implemented.

Recommended Mitigation

Below is a recommended code with the correct desired implementation. Alternatively, the check may also be removed.

- remove this code require(msg.sender == currentKing, "Game: You are already the king. No need to re-claim.");
​
+ add this code require(msg.sender != currentKing, "Game: You are already the king. No need to re-claim.");
​

The above check shows how to correctly implement the require check and ensure that the game logic plays out as originally intended,

Updates

Appeal created

inallhonesty Lead Judge about 2 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Game::claimThrone `msg.sender == currentKing` check is busted

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.