Last Man Standing

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

Unsafe Single-Step Ownership Renouncement Can Permanently Lock Privileged Functions

Root + Impact

The contract uses OpenZeppelin’s Ownable, which implements ownership transfer in a single step and allows the current owner to immediately renounce ownership by setting the owner to the zero address. This design lacks safeguards against accidental or premature renouncement of ownership before proper handover.

Description

  • Under normal circumstances, the Ownable contract from OpenZeppelin allows the current owner (typically the deployer) to transfer ownership to a new address using transferOwnership(), or to renounce ownership entirely using renounceOwnership(). These functions are intended to give the contract administrator control over privileged functionality and ownership lifecycle.

  • However, the contract currently uses Ownable, which supports allows immediate renouncement of ownership by setting the owner to the zero address. This creates a risk where the current owner might accidentally or prematurely call renounceOwnership(), resulting in permanent loss of ownership. If this happens before a new owner is properly set, the contract becomes ownerless and any onlyOwner functions become permanently inaccessible.

  • In the context of this game contract, this becomes a breaking issue: after ownership is renounced, the resetGame() function — which is restricted to the onlyOwner — becomes permanently inaccessible. As a result, once the first round of the game concludes, no further rounds can be started, effectively halting the entire game logic and rendering the contract unusable beyond the first cycle.

@> import "openzeppelin-contracts/contracts/access/Ownable.sol";

Risk

Likelihood: HIGH

  • Ownership renouncement can be triggered at any time by the owner since the Ownable contract exposes a public renounceOwnership() function that executes immediately and irreversibly.

  • There is no safeguard or intermediate confirmation step (like in Ownable2Step) to prevent mistakes or ensure safe delegation, increasing the risk of accidental or malicious renouncement.

Impact:

  • The resetGame() function becomes permanently inaccessible, preventing the game from progressing beyond the first round. Since only the owner can call this function, the contract enters a terminal state once the first game concludes.

  • The withdrawPlatformFees() function is also locked, meaning any accumulated platform fees become permanently stuck in the contract. This results in unrecoverable funds and failed platform revenue extraction.

  • Overall, ownership renouncement leads to complete loss of control over core administrative functions and a permanent denial of service for all future game rounds and financial operations.

Proof of Concept

  1. player1 claims the throne by sending 2 ether to the contract.

  2. Time is fast-forwarded to exceed the grace period (simulate end of the current round).

  3. deployer (owner) renounces ownership of the contract.

  4. player1 calls declareWinner() to end the round and become eligible for winnings.

  5. player1 calls withdrawWinnings() to successfully withdraw their reward.

  6. deployer attempts to call withdrawPlatformFees() but the call reverts because the contract has no owner anymore.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;
import {Test, console2} from "forge-std/Test.sol";
import {Game} from "../src/Game.sol";
contract GameTest is Test {
Game public game;
address public deployer;
address public player1;
address public player2;
address public player3;
address public maliciousActor;
// Initial game parameters for testing
uint256 public constant INITIAL_CLAIM_FEE = 0.1 ether; // 0.1 ETH
uint256 public constant GRACE_PERIOD = 1 days; // 1 day
uint256 public constant FEE_INCREASE_PERCENTAGE = 10; // 10%
uint256 public constant PLATFORM_FEE_PERCENTAGE = 5; // 5%
function setUp() public {
deployer = makeAddr("deployer");
player1 = makeAddr("player1");
player2 = makeAddr("player2");
player3 = makeAddr("player3");
maliciousActor = makeAddr("maliciousActor");
vm.deal(deployer, 10 ether);
vm.deal(player1, 10 ether);
vm.deal(player2, 10 ether);
vm.deal(player3, 10 ether);
vm.deal(maliciousActor, 10 ether);
vm.startPrank(deployer);
game = new Game(
INITIAL_CLAIM_FEE,
GRACE_PERIOD,
FEE_INCREASE_PERCENTAGE,
PLATFORM_FEE_PERCENTAGE
);
vm.stopPrank();
}
function testOwnershipRenounceFlow() public {
// Step 1: Player1 claims the throne with 2 ether
vm.prank(player1);
game.claimThrone{value: 2 ether}();
// Step 2: Fast forward time past claim period
vm.warp(block.timestamp + GRACE_PERIOD + 1);
// Step 3: Owner renounces ownership
vm.prank(deployer);
game.renounceOwnership();
//player1 declares winner (should succeed)
vm.prank(player1);
game.declareWinner();
// Step 4: Current king withdraws winnings (should succeed)
vm.prank(player1);
game.withdrawWinnings();
// Step 5: Owner tries to withdraw platform fees (should fail)
vm.prank(deployer);
vm.expectRevert("Ownable: caller is not the owner");
game.withdrawPlatformFees();
}
}

Recommended Mitigation

To prevent loss of critical functionality due to accidental or premature ownership renouncement:

  • Avoid calling renounceOwnership() unless the contract is explicitly designed to be fully decentralized and requires no future administrative actions.

  • If ownership renouncement is intended, ensure all necessary privileged actions (like withdrawing platform fees or resetting the game) are completed first.

  • For a more secure approach, only allow the renounceOwnership() when there has been transfer of ownership.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;
- import "openzeppelin-contracts/contracts/access/Ownable.sol";
+ import "@openzeppelin/contracts/access/Ownable2Step.sol";
- contract Game is Ownable {
+ contract Game is Ownable2Step {
// --- State Variables ---
// Game Core State
address public currentKing; // The address of the current "King"
uint256 public lastClaimTime; // Timestamp when the throne was last claimed
uint256 public gracePeriod; // Time in seconds after which a winner can be declared (e.g., 24 hours)
uint256 public pot; // Total ETH accumulated for the winner
uint256 public claimFee; // Current ETH fee required to claim the throne
bool public gameEnded; // True if a winner has been declared for the current round
+ bool public ownershipTransferred ;
// Game Parameters (Configurable by Owner)
uint256 public initialClaimFee; // The starting fee for a new game round
uint256 public feeIncreasePercentage; // Percentage by which the claimFee increases after each successful claim (e.g., 10 for 10%)
uint256 public platformFeePercentage; // Percentage of the claimFee that goes to the contract owner (deployer)
uint256 public initialGracePeriod; // The grace period set at the start of a new game round
// Payouts and Balances
mapping(address => uint256) public pendingWinnings; // Stores ETH owed to the declared winner (pot + prev king payouts)
uint256 public platformFeesBalance; // Accumulated platform fees for the contract owner
// Game Analytics/History
uint256 public gameRound; // Current round number of the game
uint256 public totalClaims; // Total number of throne claims across all rounds
mapping(address => uint256) public playerClaimCount; // How many times an address has claimed the throne in total
// Manual Reentrancy Guard
bool private _locked; // Flag to prevent reentrant calls
// --- Events ---
/**
* @dev Emitted when a new player successfully claims the throne.
* @param newKing The address of the new king.
* @param claimAmount The ETH amount sent by the new king.
* @param newClaimFee The updated claim fee for the next claim.
* @param newPot The updated total pot for the winner.
* @param timestamp The block timestamp when the claim occurred.
*/
event ThroneClaimed(
address indexed newKing,
uint256 claimAmount,
uint256 newClaimFee,
uint256 newPot,
uint256 timestamp
);
/**
* @dev Emitted when the game ends and a winner is declared.
* @param winner The address of the declared winner.
* @param prizeAmount The total prize amount won.
* @param timestamp The block timestamp when the winner was declared.
* @param round The game round that just ended.
*/
event GameEnded(
address indexed winner,
uint256 prizeAmount,
uint256 timestamp,
uint256 round
);
/**
* @dev Emitted when a winner successfully withdraws their prize.
* @param to The address that withdrew the winnings.
* @param amount The amount of ETH withdrawn.
*/
event WinningsWithdrawn(address indexed to, uint256 amount);
/**
* @dev Emitted when the contract owner withdraws accumulated platform fees.
* @param to The address that withdrew the fees (owner).
* @param amount The amount of ETH withdrawn.
*/
event PlatformFeesWithdrawn(address indexed to, uint256 amount);
/**
* @dev Emitted when a new game round is started.
* @param newRound The number of the new game round.
* @param timestamp The block timestamp when the game was reset.
*/
event GameReset(uint256 newRound, uint256 timestamp);
/**
* @dev Emitted when the grace period is updated by the owner.
* @param newGracePeriod The new grace period in seconds.
*/
event GracePeriodUpdated(uint256 newGracePeriod);
/**
* @dev Emitted when the claim fee parameters are updated by the owner.
* @param newInitialClaimFee The new initial claim fee.
* @param newFeeIncreasePercentage The new fee increase percentage.
*/
event ClaimFeeParametersUpdated(
uint256 newInitialClaimFee,
uint256 newFeeIncreasePercentage
);
/**
* @dev Emitted when the platform fee percentage is updated by the owner.
* @param newPlatformFeePercentage The new platform fee percentage.
*/
event PlatformFeePercentageUpdated(uint256 newPlatformFeePercentage);
// --- Modifiers ---
/**
* @dev Throws if the game has already ended.
*/
modifier gameNotEnded() {
require(
!gameEnded,
"Game: Game has already ended. Reset to play again."
);
_;
}
/**
* @dev Throws if the game has not yet ended.
*/
modifier gameEndedOnly() {
require(gameEnded, "Game: Game has not ended yet.");
_;
}
/**
* @dev Throws if the provided percentage is not between 0 and 100 (inclusive).
* @param _percentage The percentage value to validate.
*/
modifier isValidPercentage(uint256 _percentage) {
require(_percentage <= 100, "Game: Percentage must be 0-100.");
_;
}
/**
* @dev Prevents reentrant calls to a function.
* This is a manual implementation of a reentrancy guard.
*/
modifier nonReentrant() {
require(!_locked, "ReentrancyGuard: reentrant call");
_locked = true;
_;
_locked = false;
}
/**
* @dev Initializes the game contract.
* @param _initialClaimFee The starting fee to claim the throne.
* @param _gracePeriod The initial grace period in seconds (e.g., 86400 for 24 hours).
* @param _feeIncreasePercentage The percentage increase for the claim fee (0-100).
* @param _platformFeePercentage The percentage of claim fee for the owner (0-100).
*/
constructor(
uint256 _initialClaimFee,
uint256 _gracePeriod,
uint256 _feeIncreasePercentage,
uint256 _platformFeePercentage
) Ownable(msg.sender) {
// Validate parameters
require(
_initialClaimFee > 0,
"Game: Initial claim fee must be greater than zero."
);
require(
_gracePeriod > 0,
"Game: Grace period must be greater than zero."
);
require(
_feeIncreasePercentage <= 100,
"Game: Fee increase percentage must be 0-100."
);
require(
_platformFeePercentage <= 100,
"Game: Platform fee percentage must be 0-100."
);
initialClaimFee = _initialClaimFee;
initialGracePeriod = _gracePeriod;
feeIncreasePercentage = _feeIncreasePercentage;
platformFeePercentage = _platformFeePercentage;
// Initialize game state for the first round
claimFee = initialClaimFee;
gracePeriod = initialGracePeriod;
lastClaimTime = block.timestamp; // Game starts immediately upon deployment
gameRound = 1;
gameEnded = false;
// currentKing starts as address(0) until first claim
}
/**
* @dev Allows a player to claim the throne by sending the required claim fee.
* If there's a previous king, a small portion of the new claim fee is sent to them.
* A portion also goes to the platform owner, and the rest adds to the pot.
*/
function claimThrone() external payable gameNotEnded nonReentrant {
require(
msg.value >= claimFee,
"Game: Insufficient ETH sent to claim the throne."
);
require(
msg.sender != currentKing,
"Game: You are already the king. No need to re-claim."
);
uint256 sentAmount = msg.value;
uint256 previousKingPayout = 0;
uint256 currentPlatformFee = 0;
uint256 amountToPot = 0;
// Calculate platform fee
currentPlatformFee = (sentAmount * platformFeePercentage) / 100;
// Defensive check to ensure platformFee doesn't exceed available amount after previousKingPayout
if (currentPlatformFee > (sentAmount - previousKingPayout)) {
currentPlatformFee = sentAmount - previousKingPayout;
}
platformFeesBalance = platformFeesBalance + currentPlatformFee;
// Remaining amount goes to the pot
amountToPot = sentAmount - currentPlatformFee;
pot = pot + amountToPot;
// Update game state
currentKing = msg.sender;
lastClaimTime = block.timestamp;
playerClaimCount[msg.sender] = playerClaimCount[msg.sender] + 1;
totalClaims = totalClaims + 1;
// Increase the claim fee for the next player
claimFee = claimFee + (claimFee * feeIncreasePercentage) / 100;
emit ThroneClaimed(
msg.sender,
sentAmount,
claimFee,
pot,
block.timestamp
);
}
/**
* @dev Allows anyone to declare a winner if the grace period has expired.
* The currentKing at the time the grace period expires becomes the winner.
* The pot is then made available for the winner to withdraw.
*/
function declareWinner() external gameNotEnded {
require(
currentKing != address(0),
"Game: No one has claimed the throne yet."
);
require(
block.timestamp > lastClaimTime + gracePeriod,
"Game: Grace period has not expired yet."
);
gameEnded = true;
pendingWinnings[currentKing] = pendingWinnings[currentKing] + pot;
pot = 0; // Reset pot after assigning to winner's pending winnings
emit GameEnded(currentKing, pot, block.timestamp, gameRound);
}
/**
* @dev Allows the declared winner to withdraw their prize.
* Uses a secure withdraw pattern with a manual reentrancy guard.
*/
function withdrawWinnings() external nonReentrant {
uint256 amount = pendingWinnings[msg.sender];
require(amount > 0, "Game: No winnings to withdraw.");
(bool success, ) = payable(msg.sender).call{value: amount}("");
require(success, "Game: Failed to withdraw winnings.");
pendingWinnings[msg.sender] = 0;
emit WinningsWithdrawn(msg.sender, amount);
}
/**
* @dev Allows the contract owner to reset the game for a new round.
* Can only be called after a winner has been declared.
*/
function resetGame() external onlyOwner gameEndedOnly {
currentKing = address(0);
lastClaimTime = block.timestamp;
pot = 0;
claimFee = initialClaimFee;
gracePeriod = initialGracePeriod;
gameEnded = false;
gameRound = gameRound + 1;
// totalClaims is cumulative across rounds, not reset here, but could be if desired.
emit GameReset(gameRound, block.timestamp);
}
/**
* @dev Allows the contract owner to update the grace period.
* @param _newGracePeriod The new grace period in seconds.
*/
function updateGracePeriod(uint256 _newGracePeriod) external onlyOwner {
require(
_newGracePeriod > 0,
"Game: New grace period must be greater than zero."
);
gracePeriod = _newGracePeriod;
emit GracePeriodUpdated(_newGracePeriod);
}
/**
* @dev Allows the contract owner to update the initial claim fee and fee increase percentage.
* @param _newInitialClaimFee The new initial claim fee.
* @param _newFeeIncreasePercentage The new fee increase percentage (0-100).
*/
function updateClaimFeeParameters(
uint256 _newInitialClaimFee,
uint256 _newFeeIncreasePercentage
) external onlyOwner isValidPercentage(_newFeeIncreasePercentage) {
require(
_newInitialClaimFee > 0,
"Game: New initial claim fee must be greater than zero."
);
initialClaimFee = _newInitialClaimFee;
feeIncreasePercentage = _newFeeIncreasePercentage;
emit ClaimFeeParametersUpdated(
_newInitialClaimFee,
_newFeeIncreasePercentage
);
}
/**
* @dev Allows the contract owner to update the platform fee percentage.
* @param _newPlatformFeePercentage The new platform fee percentage (0-100).
*/
function updatePlatformFeePercentage(
uint256 _newPlatformFeePercentage
) external onlyOwner isValidPercentage(_newPlatformFeePercentage) {
platformFeePercentage = _newPlatformFeePercentage;
emit PlatformFeePercentageUpdated(_newPlatformFeePercentage);
}
/**
* @dev Allows the contract owner to withdraw accumulated platform fees.
* Uses a secure withdraw pattern with a manual reentrancy guard.
*/
function withdrawPlatformFees() external onlyOwner nonReentrant {
uint256 amount = platformFeesBalance;
require(amount > 0, "Game: No platform fees to withdraw.");
platformFeesBalance = 0;
(bool success, ) = payable(owner()).call{value: amount}("");
require(success, "Game: Failed to withdraw platform fees.");
emit PlatformFeesWithdrawn(owner(), amount);
}
/**
* @dev Returns the time remaining until the grace period expires and a winner can be declared.
* Returns 0 if the grace period has already expired or the game has ended.
*/
function getRemainingTime() public view returns (uint256) {
if (gameEnded) {
return 0; // Game has ended, no remaining time
}
uint256 endTime = lastClaimTime + gracePeriod;
if (block.timestamp >= endTime) {
return 0; // Grace period has expired
}
return endTime - block.timestamp;
}
+ // Override acceptOwnership to track that ownership has been passed once
+ function acceptOwnership() public override {
+ super.acceptOwnership();
+ ownershipTransferred = true;
+ }
+ function renounceOwnership() public override onlyOwner {
+ require(ownershipTransferred, "Must transfer ownership before renouncing");
+ super.renounceOwnership();
+ }
/**
* @dev Returns the current balance of the contract (should match the pot plus platform fees unless payouts are pending).
*/
function getContractBalance() public view returns (uint256) {
return address(this).balance;
}
receive() external payable {}
}
Updates

Appeal created

inallhonesty Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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

Give us feedback!