Last Man Standing

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

Logic Reversal in `claimThrone` Bricks the Game (No One Can Become King)

Logic Reversal in claimThrone Bricks the Game (No One Can Become King)

Description

  • Expected behaviour: claimThrone() should allow any address that is not the current king to pay claimFee and become the new king.

  • Actual issue: the contract uses the comparison msg.sender == currentKing which reverts unless the caller is already the king. At deployment currentKing is address(0), so the first call (and every subsequent call) fails, making it impossible for the game to start.

// Game.sol ‑ claimThrone()
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.");
// ^ logic error ‑ should be `!=`

Risk

Likelihood:

  • The erroneous require is executed on every claimThrone call immediately after deployment

  • Because currentKing is address(0) at deployment, 100 % of first-mover transactions revert

Impact:

  • No one can ever become king; the core game mechanic is permanently disabled

  • ETH sent directly to the contract (e.g. via receive()) becomes locked; downstream functions (declareWinner, resetGame) are unreachable, bricking the protocol

Proof of Concept

The PoC below deploys the vulnerable contract, funds a test account, and shows that any attempt to call claimThrone() immediately reverts. Running this test therefore proves that the game cannot start in its current form.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;
import {Test} from "forge-std/Test.sol";
import {Game} from "../src/Game.sol";
/// @title PoC for Critical Logic Reversal in Game.claimThrone()
/// @notice Demonstrates that no address can ever become king due to `==` comparison
contract PoCLogicReversal is Test {
Game internal game;
address internal player1;
// Game parameters (can be arbitrary for the PoC)
uint256 internal constant INITIAL_CLAIM_FEE = 0.1 ether;
uint256 internal constant GRACE_PERIOD = 1 days;
uint256 internal constant FEE_INCREASE_PERCENTAGE = 10; // 10%
uint256 internal constant PLATFORM_FEE_PERCENTAGE = 5; // 5%
function setUp() public {
// Prepare a funded player address
player1 = makeAddr("player1");
vm.deal(player1, 10 ether);
// Deploy the vulnerable Game contract
game = new Game(
INITIAL_CLAIM_FEE,
GRACE_PERIOD,
FEE_INCREASE_PERCENTAGE,
PLATFORM_FEE_PERCENTAGE
);
}
/// @dev Expect the first throne claim to revert with the specific revert reason.
function test_CannotClaimThrone_FirstPlayer() public {
// Player1 attempts to claim the throne with the correct fee
vm.prank(player1);
vm.expectRevert("Game: You are already the king. No need to re-claim.");
game.claimThrone{value: INITIAL_CLAIM_FEE}();
}
}

Recommended Mitigation

Replace the equality check with an inequality check so that only non-kings can claim the throne. The one-line fix below is storage-layout-safe and fully restores intended gameplay.

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

Appeal created

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