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

Logic Flaw Prevents Throne from Ever Being Claimed

pwnerr

Root + Impact

The claimThrone() function contains an inverted logic check that prevents any new player from ever claiming the throne, rendering the game unplayable.

Description

  • Normally, any user should be able to claim the throne by sending the required claimFee, becoming the new king, increasing the pot, and resetting the timer.

  • However, due to a faulty logic condition in the claimThrone() function, only the current king is allowed to call the function, defeating the entire purpose of the game.

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."); // @audit suppose player1 is the king and player2 wants to claimThrone, he cant because of this statement. This prevents any user to claim king
​
uint256 sentAmount = msg.value;
uint256 previousKingPayout = 0;
uint256 currentPlatformFee = 0;
uint256 amountToPot = 0;
...
}

Risk

Likelihood:

  • This occurs immediately after contract deployment when currentKing == address(0) and any player attempts the first claim.

  • This continues to occur in every game round, as no address other than the current king can ever claim the throne.

Impact:

  • The core gameplay is completely broken, no one can ever claim the throne or win.

  • The pot never grows, grace periods never matter, and the contract becomes useless after deployment.

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;
​
import {Test, console} from "../lib/forge-std/src/Test.sol";
import {Game} from "../src/Game.sol";
​
contract GameTest is Test {
Game public game;
​
address public deployer;
address public player1;
address public player2;
address public player3;
address public maliciousActor;
​
// 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 = 0; // 10%
uint256 public constant PLATFORM_FEE_PERCENTAGE = 0; // 5%
​
function setUp() public {
deployer = makeAddr("deployer");
player1 = makeAddr("player1");
player2 = makeAddr("player2");
player3 = makeAddr("player3");
maliciousActor = makeAddr("maliciousActor");
​
vm.deal(deployer, 10 ether);
vm.deal(player1, 10 ether);
vm.deal(player2, 10 ether);
vm.deal(player3, 10 ether);
vm.deal(maliciousActor, 10 ether);
​
vm.startPrank(deployer);
game = new Game(
INITIAL_CLAIM_FEE,
GRACE_PERIOD,
FEE_INCREASE_PERCENTAGE,
PLATFORM_FEE_PERCENTAGE
);
vm.stopPrank();
}
​
​
function testBecomeKing() public {
console.log(game.currentKing()); // address(0)
​
// player 1 becomes king
vm.startPrank(player1);
game.claimThrone{value: game.claimFee()}();
vm.stopPrank();
​
console.log(game.currentKing());
​
// player 2 becomes king
vm.startPrank(player2);
game.claimThrone{value: game.claimFee()}();
vm.stopPrank();
​
assertEq(game.currentKing(), player2);
}
}

Recommended Mitigation

  • Invert the check to correctly disallow only the current king from reclaiming (if necessary)

  • Or remove the check entirely to allow re-claims, as they benefit the pot and reset the timer

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

or

- require(msg.sender == currentKing, "Game: You are already the king. No need to re-claim.");
+ // Removed check to allow any player, including current king, to reclaim
​
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.