Last Man Standing
First Flight #45
Beginner FriendlyFoundrySolidity
100 EXP
View results
Previous Next
Submission Details
Impact: medium
Likelihood: medium
Invalid

Off-by-One Error in Grace Period Check Blocks Winner Declaration

r4y4n3

Root + Impact

Description

  • Normal behavior:
    The winner should be declarable as soon as the grace period expires (block.timestamp >= lastClaimTime + gracePeriod).

    Specific issue:
    The contract uses a strict greater-than comparison (>) instead of greater-than-or-equal (>=) when checking if the grace period has expired. This creates an off-by-one error where the winner cannot be declared at the exact expiry timestamp, only one second after.

// src/Game.sol
function declareWinner() external gameNotEnded {
require(currentKing != address(0), "Game: No one has claimed the throne yet.");
require(
block.timestamp > lastClaimTime + gracePeriod, // @> Bug: should be >=
"Game: Grace period has not expired yet."
);
// ...
}

Risk

Likelihood:

  • This will occur whenever someone tries to declare the winner at the exact expiry moment.

Impact:

  • Creates a one-second window where the game cannot progress.

  • Causes unnecessary transaction failures and wasted gas.

  • Can be exploited or cause user confusion about when the grace period actually ends.

Proof of Concept

The following test demonstrates the bug. At the exact grace period expiry time, declareWinner() reverts, but succeeds one second later.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;
​
import {Test} from "forge-std/Test.sol";
import {Game} from "../src/Game.sol";
​
contract GameOffByOneTest 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 testOffByOneGracePeriodCheck() public {
// Player1 claims throne
vm.startPrank(player1);
game.claimThrone{value: INITIAL_CLAIM_FEE}();
vm.stopPrank();
​
// Get the exact time when grace period should expire
uint256 claimTime = game.lastClaimTime();
uint256 gracePeriod = game.gracePeriod();
uint256 expiryTime = claimTime + gracePeriod;
​
// Warp to the exact expiry time (not past it)
vm.warp(expiryTime);
​
// This should work but will fail due to off-by-one error
vm.expectRevert("Game: Grace period has not expired yet.");
game.declareWinner();
​
// Only works if we're 1 second past expiry
vm.warp(expiryTime + 1);
game.declareWinner(); // This should succeed
}
}

Recommended Mitigation

Change the comparison from > to >= to allow winner declaration at the exact expiry time:

- require(block.timestamp > lastClaimTime + gracePeriod, "Game: Grace period has not expired yet.");
+ require(block.timestamp >= lastClaimTime + gracePeriod, "Game: Grace period has not expired yet.");
Updates

Appeal created

inallhonesty Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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

Give us feedback!