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

[H-1] Flaw in claimThrone Renders Game Unplayable

cosminmarian53

[H-1] Flaw in claimThrone Renders Game Unplayable

Description

The claimThrone function is intended to allow a new player to become the currentKing by paying a fee. This is the central mechanic of the game, allowing for a "King of the Hill" style competition.

The core logic of the claimThrone function contains an inverted require statement. Instead of checking that the claimant is not the current king, it checks that the claimant is the current king. Since the game starts with currentKing as address(0), this check always fails for the first player, preventing anyone from ever becoming king and freezing the game in its initial state.

// src/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.");
​
uint256 sentAmount = msg.value;
uint256 previousKingPayout = 0;
// ...

Risk

Likelihood: High

  • This bug occurs on the very first attempt to call claimThrone in any game round.

  • It is a certainty that every deployed instance of this contract is immediately and permanently unplayable.

Impact: High

  • The contract's core functionality is completely broken. No player can ever become the king, and the game cannot proceed past its initial state.

  • The contract fails to serve its purpose, leading to a total loss of user trust and a failure of the application.

Proof of Concept

The following Foundry test simulates a full game lifecycle and proves that the game is stuck from the beginning. It shows that the first player's attempt to claim the throne is reverted, the currentKing is never updated, and as a result, a winner can never be declared.

// test/Game.t.sol
​
function testGameStuckKingNotUpdated() public {
console2.log("--- Test Start: Verifying game is stuck ---");
​
// --- Step 1: Player 1 attempts to claim the throne and fails ---
console2.log("Attempting Player 1 claim...");
vm.startPrank(player1);
vm.expectRevert("Game: You are already the king. No need to re-claim.");
game.claimThrone{value: INITIAL_CLAIM_FEE}();
vm.stopPrank();
console2.log("Player 1 claim correctly reverted.");
​
// --- Step 2: Verify that the king was NOT updated ---
// This is the core of the issue. The king should be player1, but is still address(0).
assertEq(game.currentKing(), address(0), "BUG CONFIRMED: currentKing should be player1, but is address(0)");
console2.log("Verified currentKing is still address(0).");
​
// --- Step 3: Advance time far beyond the grace period ---
console2.log("Warping time forward by", GRACE_PERIOD + 1, "seconds...");
vm.warp(block.timestamp + GRACE_PERIOD + 1);
console2.log("Time advanced.");
​
// --- Step 4: Attempt to declare a winner ---
// This will fail because `currentKing` is `address(0)`.
// The game is now permanently stuck. No one can claim, and no winner can be declared.
console2.log("Attempting to declare a winner...");
vm.prank(player3); // Anyone can try to declare.
vm.expectRevert("Game: No one has claimed the throne yet.");
game.declareWinner();
console2.log("declareWinner correctly reverted because no one ever became king.");
assertEq(game.currentKing(), address(0), "BUG CONFIRMED: currentKing should still be address(0) after declareWinner attempt");
console2.log("--- Test End: Confirmed game is permanently stuck ---");
}

Recommended Mitigation

The logical operator in the require statement within the claimThrone function must be inverted from == to !=. This ensures that a player can only claim the throne if they are not already the current king.

// src/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.");
+ require(msg.sender != currentKing, "Game: You are already the king. No need to re-claim.");
​
uint256 sentAmount = msg.value;
uint256 previousKingPayout = 0;
Updates

Appeal created

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