Root + Impact
Description
-
According to the documentation, once the grace period expires, no other player should be able to claim the throne, and the current king should be finalized as the winner.
-
However, the claimThrone() function does not verify whether the grace period has expired. Since the gameNotEnded modifier only checks that gameEnded is false, players can continue to claim the throne even after the round should have ended — as long as declareWinner() has not yet been called.
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: High
Impact: High
-
Violation of Promised Rewards and Game Logic: Players who should rightfully win after the grace period may be overtaken, breaking the protocol’s core assumption that the last king within the grace period is the winner.
-
Player Financial Loss: The rightful winner loses access to the accumulated pot, resulting in tangible economic harm and undermining trust in the fairness of the game.
Proof of Concept
Add the following test, then run this command: forge test -vv --match-test testClaimThroneAlthoughExpired
function testClaimThroneAlthoughExpired() public {
console2.log("player1 claims the throne");
uint256 claimFee = game.claimFee();
vm.prank(player1);
game.claimThrone{value: claimFee}();
console2.log("Wait for grace period to expire");
skip(GRACE_PERIOD + 1);
console2.log("Remaining time: ", game.getRemainingTime());
claimFee = game.claimFee();
vm.prank(player2);
game.claimThrone{value: claimFee}();
console2.log("player2 successfully claims the throne although grace period has expired");
}
PoC Results:
forge test -vv --match-test testClaimThroneAlthoughExpired
[⠊] Compiling...
[⠔] Compiling 1 files with Solc 0.8.29
[⠒] Solc 0.8.29 finished in 473.13ms
Compiler run successful!
Ran 1 test for test/Game.t.sol:GameTest
[PASS] testClaimThroneAlthoughExpired() (gas: 206162)
Logs:
player1 claims the throne
Wait for grace period to expire
Remaining time: 0
player2 successfully claims the throne although grace period has expired
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 6.07ms (1.52ms CPU time)
Ran 1 test suite in 233.18ms (6.07ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Recommended Mitigation
Add the check that whether the grace period expires
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: Grace period has expired."
+ );
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
);
}
