Last Man Standing

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

lastClaimTime Updated Incorrectly in resetGame()

Root + Impact

Description

  • Normal behavior:
    The grace period timer (lastClaimTime) should only start when someone actually claims the throne, not when the game resets.

    Specific issue:
    In resetGame(), lastClaimTime is set to block.timestamp while currentKing is set to address(0). This creates an inconsistent state where the grace period timer is running even though no one has claimed the throne yet.

// src/Game.sol
function resetGame() external onlyOwner gameEndedOnly {
currentKing = address(0); // No king exists
lastClaimTime = block.timestamp; // @> Bug: Grace period starts immediately
pot = 0;
claimFee = initialClaimFee;
gracePeriod = initialGracePeriod;
gameEnded = false;
gameRound = gameRound + 1;
}

Risk

Likelihood:

  • This occurs every time resetGame() is called to start a new round.

Impact:

  • Creates inconsistent state where grace period runs without a king.

  • getRemainingTime() shows countdown when no one has claimed the throne.

  • Confusing behavior where grace period can "expire" before anyone claims.

  • Potential for confusing error messages when calling declareWinner().

Proof of Concept

The following tests demonstrate the inconsistent state created by incorrect lastClaimTime setting:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;
import {Test} from "forge-std/Test.sol";
import {Game} from "../src/Game.sol";
contract GameLastClaimTimeTest is Test {
Game public game;
address public deployer;
address public player1;
uint256 public constant INITIAL_CLAIM_FEE = 0.1 ether;
uint256 public constant GRACE_PERIOD = 1 days;
uint256 public constant FEE_INCREASE_PERCENTAGE = 10;
uint256 public constant PLATFORM_FEE_PERCENTAGE = 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 testResetGameSetsIncorrectLastClaimTime() public {
// Player1 claims throne first
vm.startPrank(player1);
game.claimThrone{value: INITIAL_CLAIM_FEE}();
vm.stopPrank();
// End the game
vm.warp(block.timestamp + GRACE_PERIOD + 1);
game.declareWinner();
// Get timestamp before reset
uint256 resetTimestamp = block.timestamp;
// Reset the game
vm.startPrank(deployer);
game.resetGame();
vm.stopPrank();
// Check the inconsistent state after reset
assertEq(game.currentKing(), address(0), "No king should exist after reset");
assertEq(game.lastClaimTime(), resetTimestamp, "lastClaimTime incorrectly set to reset time");
// This creates an inconsistent state:
// - No king exists (currentKing = address(0))
// - But grace period timer is already running (lastClaimTime = block.timestamp)
}
function testGetRemainingTimeWithoutKing() public {
// Player1 claims throne first
vm.startPrank(player1);
game.claimThrone{value: INITIAL_CLAIM_FEE}();
vm.stopPrank();
// End and reset the game
vm.warp(block.timestamp + GRACE_PERIOD + 1);
game.declareWinner();
vm.startPrank(deployer);
game.resetGame();
vm.stopPrank();
// getRemainingTime() returns a value even though no king exists
uint256 remainingTime = game.getRemainingTime();
assertTrue(remainingTime > 0, "getRemainingTime should be 0 when no king exists");
// This is problematic because it shows a countdown when no one has claimed
}
function testGracePeriodRunsWithoutKing() public {
// Start fresh game
vm.startPrank(player1);
game.claimThrone{value: INITIAL_CLAIM_FEE}();
vm.stopPrank();
vm.warp(block.timestamp + GRACE_PERIOD + 1);
game.declareWinner();
vm.startPrank(deployer);
game.resetGame();
vm.stopPrank();
// Fast forward past grace period without anyone claiming
vm.warp(block.timestamp + GRACE_PERIOD + 1);
// The grace period check would pass, but declareWinner fails due to no king
vm.expectRevert("Game: No one has claimed the throne yet.");
game.declareWinner();
// This shows the inconsistent state: grace period expired but no king exists
}
}

Recommended Mitigation

Set lastClaimTime to 0 in resetGame() and only update it when someone actually claims the throne:

function resetGame() external onlyOwner gameEndedOnly {
currentKing = address(0);
- lastClaimTime = block.timestamp;
+ lastClaimTime = 0;
pot = 0;
claimFee = initialClaimFee;
gracePeriod = initialGracePeriod;
gameEnded = false;
gameRound = gameRound + 1;
}
Updates

Appeal created

inallhonesty Lead Judge 15 days ago
Submission Judgement Published
Validated
Assigned finding tags:

Game gets stuck if no one claims

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.