Last Man Standing

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

Flawed Access Control in claimThrone Prevents King from Being Dethroned, Breaking Core Game Logic

Root + Impact

Description

  • The game is intended to be a "last man standing" contest where players can usurp the current king by paying a fee. This allows for continuous competition until a final winner is determined after a grace period.

  • The claimThrone function has an inverted logic check that requires the caller to already be the king. This critical flaw prevents any new player from ever claiming the throne, permanently locking the kingship to the first person who claims it.

// Root cause in the codebase with <r> marks to highlight the relevant section
function claimThrone() external payable gameNotEnded nonReentrant {
<r>require(msg.sender == currentKing, "Game: You are already the king. No need to re-claim.");</r>
uint256 sentAmount = msg.value;
// ...
}

Risk

Likelihood:

  • The vulnerability manifests the moment the first player calls claimThrone() and becomes currentKing.

  • Every subsequent call to claimThrone() by any other player is guaranteed to revert, as the caller's address will not match the currentKing's address.

Impact:

  • The core game mechanic is broken. The "last man standing" game becomes a "first man wins" game, as no one can challenge the initial king.

  • The first player to claim the throne is guaranteed to win the entire prize pot, completely centralizing the outcome and removing any element of competition or fairness.

Proof of Concept

The following Proof of Concept can be added as a test to test/Game.t.sol. It demonstrates the vulnerability by simulating the exact exploit scenario:

  1. player1 successfully calls claimThrone() to become the first king.

  2. player2 then attempts to claim the throne by sending the new, increased fee.

  3. The test asserts that player2's transaction reverts. This is because the flawed logic require(msg.sender == currentKing) blocks any user who is not already the king.

  4. This test proves that the kingship is locked to the first claimant, rendering the game unplayable for all other participants.

// test/Game.t.sol
function test_PoC_KingCanNeverBeDethroned() public {
// Player 1 becomes the first king.
vm.startPrank(player1);
game.claimThrone{value: INITIAL_CLAIM_FEE}();
vm.stopPrank();
assertEq(game.currentKing(), player1);
// Player 2 attempts to claim the throne.
uint256 newFee = game.claimFee();
vm.startPrank(player2);
// The transaction is expected to revert because of the flawed check.
// The error message itself is misleading, but it's the one in the code.
vm.expectRevert("Game: You are already the king. No need to re-claim.");
game.claimThrone{value: newFee}();
vm.stopPrank();
// Verify Player 1 is still the king.
assertEq(game.currentKing(), player1, "The king should not have changed.");
}

Recommended Mitigation

The fix is to correct the inverted logic in the require statement within the claimThrone function. The comparison operator should be changed from == (equals) to != (not equals). This change ensures that a transaction will only proceed if the sender is not the current king. This correctly prevents the king from reclaiming the throne from themselves while allowing any other challenger to do so. This single change restores the core "King of the Hill" mechanic and makes the game playable as intended.

- 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.