Last Man Standing

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

[H-1] New Player Can Never Enter Game Due to Inverted Claim Logic

Description

The claimThrone() function's access control check is inverted:

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

This causes:

  1. First claims to revert (when currentKing == address(0))

  2. Incorrect error message about reclaims

  3. Complete lockout of new players

Risk

Likelihood:

  • The condition will always fail for first-time claimants since currentKing is initialized to address(0)

  • The revert will trigger on every initial claim attempt due to the inverted logic

Impact:

  • Permanently prevents the game from starting as no player can become the first king

  • Locks all ETH sent to claimThrone() since claims can never succeed

Proof of Concept

function testFirstClaimAlwaysReverts() public {
// new player
address player = makeAddr("player");
// feeding some ether to participate
vm.deal(player, 5 ether);
// getting initial claimFee to enter the game
uint256 claimFee = game.claimFee();
console.log("current king before player enters :", game.currentKing());
vm.prank(player);
vm.expectRevert("Game: You are already the king. No need to re-claim.");
// trying to claim the throne
game.claimThrone{value: claimFee}();
console.log("current king:", game.currentKing());
}
  1. Initial State Check: Confirms no king exists before test (address(0))

  2. Fund Preparation: Player gets 5 ETH to cover claim fee

  3. Revert Verification: Confirms the inverted condition triggers wrong revert

  4. State Preservation: Shows game remains in initial unusable state

Logs

forge test --mt testFirstClaimAlwaysReverts -vvv
[⠊] Compiling...
No files changed, compilation skipped
Ran 1 test for test/Game.t.sol:GameTest
[PASS] testFirstClaimAlwaysReverts() (gas: 56035)
Logs:
current king before player enters : 0x0000000000000000000000000000000000000000
current king: 0x0000000000000000000000000000000000000000
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.28ms (271.89µs CPU time)
Ran 1 test suite in 6.62ms (1.28ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Recommended Mitigation

- 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.");
  1. Logical Correction: Changes == to != to properly:

    • Allow first claims when currentKing == address(0)

    • Block actual reclaim attempts by current king

  2. Message Preservation: Keeps same error message which now correctly describes reclaim prevention

  3. Minimal Change: Single-character edit fixes core functionality without introducing new risks

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.