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

[C-01] - Backwards logic in claimThrone() function prevents all game participation

alexandercgkarlsson

C-01. Backwards logic in claimThrone() function prevents all game participation

Root Cause + Impact

Description

The claimThrone() function in Game.sol contains a critical logic error in its access control mechanism that prevents any player from successfully claiming the throne, effectively making the entire game non-functional.

The function implements a require statement that checks if the caller is the current king, but the logic is completely backwards:

require(msg.sender == currentKing, "Game: You are already the king. No need to re-claim.");

This condition prevents the intended behavior because:

  • Initially, currentKing = address(0) (zero address)

  • When a player calls claimThrone(), msg.sender will never equal address(0)

  • The require statement fails, preventing any throne claims

  • The game becomes completely unusable

Risk

Likelihood:

  • HIGH - This bug affects every single call to claimThrone() without exception

  • The bug is triggered immediately upon any attempt to claim the throne

  • Rendering the game unusable

Impact:

  • CRITICAL - The entire game functionality is rendered in an unusable state

  • All ETH sent to the contract becomes permanently locked

Proof of Concept

The following test demonstrates that no player can claim the throne due to the backwards logic in the require statement.

Test Explanation:

  1. Initial State: The game starts with currentKing = address(0) and pot = 0

  2. Claim Attempt: Player1 tries to claim the throne with the required ETH fee

  3. Expected Behavior: The claim should succeed and Player1 should become the new king

  4. Actual Behavior: The transaction reverts with "Game: You are already the king. No need to re-claim."

  5. State Verification: The game state remains unchanged - no king is set and no ETH is added to the pot

function testNooneCanClaimTheThrone() public payable {
// Current King is address(0) in initialized state
// The pot is 0 in initialized state
​
console2.log("Initialized pot", game.pot());
console2.log("Current King:", game.currentKing());
​
// Player 1 tries to enter the game with claimThrone()
// @notice but get reverted with Game: You are already the king. No need to re-claim.
vm.prank(player1);
vm.expectRevert("Game: You are already the king. No need to re-claim.");
game.claimThrone{value: INITIAL_CLAIM_FEE}();
​
address currentKing = game.currentKing();
uint256 currentPot = game.pot();
​
// Current king is still address(0)
// Current pot is still 0
console2.log("Current King:", currentKing);
console2.log("Current Pot:", currentPot);
​
assertEq(game.currentKing(), address(0));
assertEq(game.pot(), 0);
}

Recommended Mitigation

The core issue is a fundamental logic error in the function control mechanism. The current implementation prevents the intended behavior by requiring the caller to BE the current king, when it should prevent the current king from re-claiming.

The require statement should be corrected to:

- 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.");

This ensures that:

  1. The current king cannot re-claim the throne

  2. New players can successfully claim the throne

  3. The game functions as intended

This fix is critical for the game to function at all and should be implemented immediately.

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.