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

Denial of Service Due to Faulty Logic in claimThrone()

alexwebbin

Root + Impact

Description

  • The claimThrone() function allows any player (other than the current king) to pay the required fee and become the new king of the game, increasing the pot and claim fee each time. The game is intended to continue until the grace period expires, at which point the last king is declared the winner.

  • Due to a poorly implemented require check in the claim throne function, the king will always be the zero address, and only the zero address can call the claim throne function, leading to denial of service for other users.

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:

  • This issue will occur anytime, and will be noticed right after deployment, as no one would be able to claim the throne due to the current king beind the zero address at deployment.

  • Even if the owner resets the game, the same issue still occurs as no one would be ever be able to claim the throne

Impact:

  • Only the current king can call claimThrone(), which prevents other players from participating and claiming the throne, and halts the game.

  • The game will not progress normally, as the pot and platform fees will always be empty as no user would be able to play the game, and the owner has no earnings.

Proof of Concept

Notice that everytime a player calls claimThrone, it will give an error. At deployment, the current king address is set to the zero address.

function testNoOneCanPlayGameDueToBadRequireLogic() public {
// Step 1: deployer sets up the game
vm.startPrank(deployer);
vm.stopPrank();
​
// Step 2: player1 attempts to claim throne
vm.startPrank(player1);
vm.expectRevert();//The game will revert as no one can call clainthrone
game.claimThrone{value: INITIAL_CLAIM_FEE}();
//Notice that the game does not allow anyone to claim the throne, and the king is still the zero address
assert(game.currentKing() != player1);
console2.log(
"Player1 attempted to claim the throne, but the king is still the zero address:",
game.currentKing()
);
}

Recommended Mitigation

  1. Correct the logic so that the msg.sender is not the current king, and thus only the king will be denied service.

  2. Optionally, we can remove the entire line, as the current king can keep sending to reclaim the throne, afterall it is the king's loss as the claimfee increases whenever claimThrone() is called.

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