Missing claimPeriod check, allows late throne claims.
Description
-
By default, a player should only be able to claim the throne only within the acceptable claimPeriod.
-
However, the contract lacks a check to ensure the claim period is still valid, allowing players to claim the throne even after the deadline.
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;
currentPlatformFee = (sentAmount * platformFeePercentage) / 100;
if (currentPlatformFee > (sentAmount - previousKingPayout)) {
currentPlatformFee = sentAmount - previousKingPayout;
}
platformFeesBalance = platformFeesBalance + currentPlatformFee;
amountToPot = sentAmount - currentPlatformFee;
pot = pot + amountToPot;
currentKing = msg.sender;
lastClaimTime = block.timestamp;
playerClaimCount[msg.sender] = playerClaimCount[msg.sender] + 1;
totalClaims = totalClaims + 1;
claimFee = claimFee + (claimFee * feeIncreasePercentage) / 100;
emit ThroneClaimed(
msg.sender,
sentAmount,
claimFee,
pot,
block.timestamp
);
}
Risk
Likelihood:
-
The issue occurs when a player calls claimThrone() after the claim period has elapsed, because the contract does not automatically prevent it.
-
It happens when the declareWinner button is not called in time, allowing players to continue claiming even though the round should have ended.
Impact:
-
Players can bypass the intended time limit, undermining the game’s fairness and expected flow.
-
It can result in conflicting outcomes, where the rightful winner is overridden by late claimants, possibly leading to disputes or financial loss.
Proof of Concept
Deploy the contract.
Make sure to set an initial gracePeriod value (e.g., 30 seconds).
Player A claims the throne.
Wait for the grace period to elapse.
Player B claims the throne after time has passed.
Observe that the claim succeeds.
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;
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");
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);
game = new Game(
INITIAL_CLAIM_FEE,
GRACE_PERIOD,
FEE_INCREASE_PERCENTAGE,
PLATFORM_FEE_PERCENTAGE
);
vm.stopPrank();
}
function testClaimThroneAfterTimeElapsedShouldRevert() public {
vm.startPrank(player2);
game.claimThrone{value: 0.1 ether}();
uint256 skipSeconds = game.gracePeriod() + 1;
vm.warp(block.timestamp + skipSeconds);
vm.startPrank(player3);
vm.expectRevert();
game.claimThrone{value: 0.11 ether}();
}
}
Recommended Mitigation
To prevent this from happening, we need to add a require check in the claimThrone() function to prevent claims after the gracePeriod has elapsed:
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."
);
+ require(
+ block.timestamp <= lastClaimTime + gracePeriod,
+ "Game: Claim period has expired. Wait for the next round."
);
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
);
}
