Last Man Standing

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

Contract Completely Non Functional: No Player Can Ever Claim Throne

H-01. Contract Completely Non Functional: No Player Can Ever Claim Throne

Description

  • Describe the normal behavior in one or more sentences

In a "King of the Hill" game, any player should be able to challenge the current king by sending ETH to become the new king, while the current king should be prevented from reclaiming the throne unnecessarily.

  • Explain the specific issue or problem in one or more sentences

The claimThrone() function in the Game contract contains logic that only allows the current king to claim the throne (msg.sender == currentKing). However, since the contract initializes with currentKing = address(0) and no user can ever have msg.sender == address(0), the function will always revert for every player from the very first call. This makes the entire contract completely non-functional from deployment.

Affected Function: claimThrone() in the Game contract

// Root cause in the codebase with @> marks to highlight the relevant section
// Root cause: Logic requires msg.sender to be address(0) which is impossible
require(msg.sender == currentKing, "Game: You are already the king. No need to re-claim.");
// @> currentKing initializes to address(0), but msg.sender can never be address(0)
// @> Should be: msg.sender != currentKing to allow actual players to claim throne

Risk

Likelihood:

  • This bug occurs on every single call to claimThrone() from the moment the contract is deployed, making it 100% reproducible with zero exceptions

  • The contract initializes with currentKing = address(0) and no user can ever have this address as msg.sender, making the bug mathematically impossible to avoid

Impact:

  • Complete contract failure - the protocol is entirely non-functional from deployment and no user can ever interact with its core functionality.

  • Total economic loss for deployers and complete waste of deployment costs, as the contract serves no purpose and generates zero revenue

Proof of Concept

The test demonstrates that the contract is completely non-functional from deployment. Since currentKing initializes to address(0) and no user can ever have msg.sender == address(0), the require statement will always fail for every player attempting to claim the throne.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;
import {Test, console2} from "forge-std/Test.sol";
import {Game} from "../src/Game.sol";
contract GameTest is Test {
Game public game;
address public deployer;
address public player1;
// Initial game parameters for testing
uint256 public constant INITIAL_CLAIM_FEE = 0.1 ether; // 0.1 ETH
uint256 public constant GRACE_PERIOD = 1 days; // 1 day in seconds
uint256 public constant FEE_INCREASE_PERCENTAGE = 10; // 10%
uint256 public constant PLATFORM_FEE_PERCENTAGE = 5; // 5%
function setUp() public {
deployer = makeAddr("deployer");
player1 = makeAddr("player1");
vm.deal(deployer, 10 ether);
vm.deal(player1, 10 ether);
vm.startPrank(deployer);
game = new Game(INITIAL_CLAIM_FEE, GRACE_PERIOD, FEE_INCREASE_PERCENTAGE, PLATFORM_FEE_PERCENTAGE);
vm.stopPrank();
}
function testClaimThroneReverts() public {
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 test demonstrates that the contract is completely non-functional from deployment. Since currentKing initializes to address(0) and no user can ever have msg.sender == address(0), the require statement will always fail for every player attempting to claim the throne.

Recommended Mitigation

Change the boolean logic in the require statement to allow players other than the current king to claim the throne:

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