Last Man Standing

First Flight #45
Beginner FriendlyFoundrySolidity
100 EXP
View results
Submission Details
Severity: medium
Valid

Game::claimThrone function contains wrong condition in require statement, that makes complete game unplayable

Game::claimThrone function contains wrong condition in require statement, that makes complete game unplayable

Description

  • Game::claimThrone function allows a player to claim the throne. Function has an require statement that contains wrong condition about the function caller and makes complete game unplayable.

  • The expected check should be, function caller should not be the currentKing

  • Current code implements, caller should be the currentKing to call the claimThrone function

  • This wrong condition in require statement makes the complete game unplayable for other player

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: High

  • Other player trying to claimThrone will not able to participate in the game due to wrong require condition. Game will become unplayable for all other players. According to current code, only currentKing will able to claimThrone and participate in the game

Impact: High

  • Game will become unplayable for all other players except the currentKing according to current code implementation

Proof of Concept

  • Add test\_claimThrone\_Exploit function in Game.t.sol file. To run the test run the command forge test --mt test\_claimThrone\_Exploit -vvvv

  • The test will depect, players other than currentKing will not be able to claimThrone

function test_claimThrone_Exploit() public {
assert(game.currentKing() == address(0));
assert(game.platformFeesBalance() == 0);
assert(game.pot() == 0);
vm.startPrank(user1);
vm.expectRevert("Game: You are already the king. No need to re-claim.");
game.claimThrone{value:2 ether}();
vm.stopPrank();
}

Recommended Mitigation

  • To make the game playable make the changes as given below

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;
// 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
);
}
Updates

Appeal created

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