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

Incorrect Condition in claimThrone Function Prevents Game Functionality

samuelsmith442

Description

The Last Man Standing game has a critical vulnerability in its core claimThrone() function that completely breaks the game's functionality. The function contains an incorrect condition check that prevents any player from ever claiming the throne.

Expected Behavior

Players should be able to claim the throne by sending the required ETH amount, as long as they are not already the current king.

Actual Behavior

The condition in claimThrone() is reversed, requiring that the sender must already be the current king to claim the throne. Since the initial king is set to address(0), no real address can ever satisfy this condition, making the game completely unplayable.

Root Cause

The vulnerability exists in the claimThrone() function where the condition check uses == instead of !=:

// Line 188 in Game.sol
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.");
// Rest of the function...
}

The error message "You are already the king. No need to re-claim" indicates that the intention was to prevent the current king from claiming the throne again. However, the condition msg.sender == currentKing does the opposite - it only allows the current king to claim the throne.

Risk Assessment

Impact

The impact is catastrophic as it completely breaks the core functionality of the game:

  1. No player can ever claim the throne

  2. The game cannot progress beyond its initial state

  3. No pot can be accumulated

  4. No winner can ever be declared

Likelihood

The likelihood is extremely high as this issue will be encountered by the very first player who attempts to interact with the game. It's not an edge case but affects the primary function of the contract.

Proof of Concept

The following test demonstrates the issue:

function testVulnerability_ClaimThroneIncorrectCondition() public {
// Initial state: currentKing is address(0)
assertEq(game.currentKing(), address(0));
// Player1 tries to claim the throne, but will fail due to the incorrect condition
vm.startPrank(player1);
vm.expectRevert("Game: You are already the king. No need to re-claim.");
game.claimThrone{value: INITIAL_CLAIM_FEE}();
vm.stopPrank();
// The currentKing should still be address(0)
assertEq(game.currentKing(), address(0));
}

Console Output

[PASS] testVulnerability_ClaimThroneIncorrectCondition() (gas: 50130)

Recommended Mitigation

The condition should be reversed to check that the sender is NOT the current king:

// Before: Vulnerable 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.");
// Rest of the function...
}
​
// After: Fixed 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.");
// Rest of the function...
}

Explanation

The fix changes the equality check (==) to an inequality check (!=), which correctly implements the intended behavior of preventing the current king from claiming the throne again. This allows new players to claim the throne while preventing the current king from reclaiming unnecessarily.

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.