Description
The claimThrone function allows players to claim the throne by sending ETH greater than or equal to the current claimFee, updating the currentKing, increasing the pot, and incrementing the claimFee for the next claim. A platform fee is deducted from the sent amount, and the game state is updated with the new king and claim details.
The function contains a critical bug in the require statement that checks if the caller is the currentKing. The condition require(msg.sender == currentKing, ...) incorrectly allows only the currentKing to claim the throne again, but since currentKing is initialized as address(0), no player can ever satisfy this condition, preventing any claims and causing a complete Denial of Service (DoS) that renders the game unplayable.
function claimThrone() external payable gameNotEnded nonReentrant {
require(msg.value >= claimFee, "Game: Insufficient ETH sent to claim the throne.");
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 immediately upon deployment when any player attempts to call claimThrone, as currentKing is initialized to address(0).
-
Every subsequent attempt to claim the throne by any player will fail, as no player’s address can match address(0).
Impact:
-
The game is completely unplayable, as no player can claim the throne, preventing the game from starting or progressing.
-
Players lose trust in the contract, and any ETH sent in failed transactions is wasted on gas fees, discouraging participation.
Proof of Concept
The following unit test demonstrates the critical DoS vulnerability in the claimThrone function, where the incorrect require(msg.sender == currentKing, ...) statement prevents any player from claiming the throne, rendering the game unplayable. The test is designed to show that both an initial and subsequent claim attempts fail, leaving the game state unchanged and confirming the total lockout of the game.
Paste this unit test in the test file:
function testDenialOfServiceNoPlayerCanClaimThrone() public {
vm.prank(player1);
vm.expectRevert("Game: You are already the king. No need to re-claim.");
game.claimThrone{value: INITIAL_CLAIM_FEE}();
assertEq(game.currentKing(), address(0), "Current king should still be address(0)");
assertEq(game.pot(), 0, "Pot should still be 0");
assertEq(game.claimFee(), INITIAL_CLAIM_FEE, "Claim fee should not change");
assertEq(game.playerClaimCount(player1), 0, "Player1 should still have no claims");
assertEq(game.totalClaims(), 0, "Total claims should still be 0");
vm.prank(player2);
vm.expectRevert("Game: You are already the king. No need to re-claim.");
game.claimThrone{value: INITIAL_CLAIM_FEE}();
assertEq(game.currentKing(), address(0), "Current king should still be address(0)");
assertEq(game.pot(), 0, "Pot should still be 0");
assertEq(game.claimFee(), INITIAL_CLAIM_FEE, "Claim fee should not change");
assertEq(game.playerClaimCount(player2), 0, "Player2 should have no claims");
assertEq(game.totalClaims(), 0, "Total claims should still be 0");
}
## Recommended Mitigation
Correct the `require` statement to prevent the current king from re-claiming the throne, allowing new players to claim it. This ensures the first player can claim when `currentKing` is `address(0)` and subsequent players can claim when they are not the `currentKing`.
```diff
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(msg.sender != currentKing, "Game: You are already the king. No need to re-claim.");
uint256 sentAmount = msg.value;
}
