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

Game Unplayable due to Incorrect Check

iafrika

Inverted Logic in claimThrone() Makes Game Completely Unplayable

Description

The claimThrone() function should allow any player except the current king to claim the throne, creating a competitive "King of the Hill" game where players can overthrow each other by paying the required fee.

The function contains an inverted logic check that only allows the current king to claim the throne again, which completely breaks the game mechanics. Since currentKing starts as address(0) and no real address can equal address(0), no player can ever make the first claim, rendering the entire game permanently unplayable.

Root Cause

https://github.com/CodeHawks-Contests/2025-07-last-man-standing/blob/47d9d19a78acb52270269f4bff1568b87eb81a96/src/Game.sol#L188

// Root cause in the codebase with @> marks to highlight the relevant section
​
​
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 function
}

Risk

Likelihood:

  • Every single attempt to call claimThrone() will fail due to the inverted logic check

  • The issue occurs immediately upon contract deployment when any player tries to make the first claim

Impact:

  • Complete game failure - no player can ever claim the throne or participate in the game

  • All ETH sent with claimThrone() calls will be reverted, but gas costs are still incurred by users

  • Contract becomes entirely non-functional despite successful deployment

  • Misleading error messages confuse users ("You are already the king" when they're not the king)

Proof of Concept

function test_GameUnplayable_IncorrectCheck() public {
// BUG EXPLANATION:
// The check require(msg.sender == currentKing) means "only the current king can claim"
// Initially currentKing = address(0), so only address(0) could claim
// But address(0) cannot send transactions, making the game unplayable
// Step 1: Verify initial game state
assertEq(game.currentKing(), address(0)); // No king initially
assertEq(game.totalClaims(), 0); // No claims made yet
// Step 2: Demonstrate that Player1 cannot claim
// msg.sender = player1 (some real address like 0x123...)
// currentKing = address(0)
// Check: require(0x123... == 0x000...) → FALSE → REVERT
vm.prank(player1);
vm.expectRevert("Game: You are already the king. No need to re-claim.");
game.claimThrone{value: INITIAL_CLAIM_FEE}();
// Step 3: Demonstrate that Player2 also cannot claim
// Same logic applies - no real address equals address(0)
vm.prank(player2);
vm.expectRevert("Game: You are already the king. No need to re-claim.");
game.claimThrone{value: INITIAL_CLAIM_FEE}();
// Step 4: Even the contract deployer cannot claim
// Proves that NO address can ever claim, regardless of permissions
vm.prank(deployer);
vm.expectRevert("Game: You are already the king. No need to re-claim.");
game.claimThrone{value: INITIAL_CLAIM_FEE}();
// Step 5: Verify the game state remains unchanged
// This proves that the bug prevents any progress in the game
assertEq(game.currentKing(), address(0)); // Still no king
assertEq(game.totalClaims(), 0); // Still no successful claims
assertEq(game.pot(), 0); // Pot remains empty
// SUMMARY: The inverted logic makes the game completely unplayable
// because no real Ethereum address can ever equal address(0)
}

Recommended Mitigation

The fix is simple but critical - change the equality operator from == to !=:

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.");
// ... rest of function remains unchanged
}

Explanation of the fix:

  1. Before fix: msg.sender == currentKing means "only the current king can claim"

    • Initially: only address(0) can claim (impossible)

    • After first claim: only the current king can claim again (wrong game logic)

  2. After fix: msg.sender != currentKing means "anyone except the current king can claim"

    • Initially: any real address can claim (correct - allows first player to start the game)

    • After first claim: any player except the current king can claim (correct - enables competition)

  3. Game flow with fix:

  4. Player1 claims → becomes king (Player1 != address(0) = TRUE)

  5. Player2 can claim from Player1 → becomes king (Player2 != Player1 = TRUE)

  6. Player1 cannot immediately re-claim (Player1 == Player1 = FALSE)

  7. Player3 can claim from Player2 → becomes king (Player3 != Player2 = TRUE)

This single character change transforms the contract from completely broken to fully functional, enabling the intended "King of the Hill" competitive gameplay.

Updates

Appeal created

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

Give us feedback!