Last Man Standing

First Flight #45
Beginner FriendlyFoundrySolidity
100 EXP
View results
Submission Details
Impact: low
Likelihood: low
Invalid

Lost Stats: Counters Never Reset and Use Oversized Types

Root + Impact

Description

  • Normal behavior:
    Game statistics should be accurate, manageable, and resettable as needed. Counters should use appropriately sized types and be reset when starting new rounds to prevent indefinite accumulation.

    Specific issue:
    The contract uses uint256 for counters like totalClaims and playerClaimCount, which are never reset and accumulate across all rounds. This can theoretically lead to overflow and loss of statistical data, and makes analytics less meaningful over time.

// src/Game.sol
uint256 public totalClaims; // Never reset, accumulates indefinitely
mapping(address => uint256) public playerClaimCount; // Never reset, accumulates indefinitely
function claimThrone() external payable gameNotEnded nonReentrant {
// ...
totalClaims = totalClaims + 1;
playerClaimCount[msg.sender] = playerClaimCount[msg.sender] + 1;
// ...
}

Risk

Likelihood:

  • Overflow is extremely unlikely, but indefinite accumulation is guaranteed.

Impact:

  • Loss of statistical integrity if overflow occurs.

  • Analytics and leaderboards become less meaningful over time.

  • Unnecessary use of large storage types increases gas costs.

Proof of Concept

The following tests demonstrate the issue. Stats persist and accumulate across multiple rounds and are never reset:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;
import {Test, console2} from "forge-std/Test.sol";
import {Game} from "../src/Game.sol";
contract GameStatsOverflowTest 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, 100 ether);
vm.deal(player1, 100 ether);
vm.startPrank(deployer);
game = new Game(
INITIAL_CLAIM_FEE,
GRACE_PERIOD,
FEE_INCREASE_PERCENTAGE,
PLATFORM_FEE_PERCENTAGE
);
vm.stopPrank();
}
function testStatsNeverReset() public {
// Player1 claims throne multiple times across different rounds
vm.startPrank(player1);
game.claimThrone{value: INITIAL_CLAIM_FEE}();
vm.stopPrank();
// Check initial stats
uint256 initialTotalClaims = game.totalClaims();
uint256 initialPlayerClaims = game.playerClaimCount(player1);
assertEq(initialTotalClaims, 1, "Should have 1 total claim");
assertEq(initialPlayerClaims, 1, "Player should have 1 claim");
// End the game and reset
vm.warp(block.timestamp + GRACE_PERIOD + 1);
game.declareWinner();
vm.startPrank(deployer);
game.resetGame();
vm.stopPrank();
// Stats are NOT reset after game reset
assertEq(game.totalClaims(), initialTotalClaims, "Total claims should persist after reset");
assertEq(game.playerClaimCount(player1), initialPlayerClaims, "Player claims should persist after reset");
// Multiple rounds accumulate stats indefinitely
vm.startPrank(player1);
game.claimThrone{value: INITIAL_CLAIM_FEE}();
vm.stopPrank();
// Stats continue to accumulate
assertEq(game.totalClaims(), 2, "Total claims should accumulate");
assertEq(game.playerClaimCount(player1), 2, "Player claims should accumulate");
}
function testCountersUseUint256() public view {
// This test shows that counters use uint256, which could theoretically overflow
// While practically impossible, using smaller types would be more appropriate
// uint256 max value is extremely large, but still finite
uint256 maxValue = type(uint256).max;
console2.log("uint256 max value:", maxValue);
// For comparison, uint64 would still allow 18+ quintillion claims
uint64 maxUint64 = type(uint64).max;
console2.log("uint64 max value:", maxUint64);
// This demonstrates that uint64 would be sufficient for totalClaims
// and uint32 would be sufficient for individual player claim counts
}
function testStatsPersistenceAcrossMultipleRounds() public {
uint256 rounds = 3;
for (uint256 i = 0; i < rounds; i++) {
// Player claims throne
vm.startPrank(player1);
game.claimThrone{value: INITIAL_CLAIM_FEE}();
vm.stopPrank();
// End round
vm.warp(block.timestamp + GRACE_PERIOD + 1);
game.declareWinner();
// Reset game (except for last iteration)
if (i < rounds - 1) {
vm.startPrank(deployer);
game.resetGame();
vm.stopPrank();
}
}
// All claims are accumulated, never reset
assertEq(game.totalClaims(), rounds, "Total claims should accumulate across all rounds");
assertEq(game.playerClaimCount(player1), rounds, "Player claims should accumulate across all rounds");
}
}

Recommended Mitigation

  • Use smaller counter types (uint64 for totalClaims, uint32 for playerClaimCount)

  • Add a reset mechanism for stats when starting a new round

- uint256 public totalClaims;
+ uint64 public totalClaims;
- mapping(address => uint256) public playerClaimCount;
+ mapping(address => uint32) public playerClaimCount;
function resetGame() external onlyOwner gameEndedOnly {
// ... existing code ...
+ totalClaims = 0;
+ // Optionally reset playerClaimCount mapping if feasible
}
Updates

Appeal created

inallhonesty Lead Judge 26 days ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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