Last Man Standing
First Flight #45
Beginner FriendlyFoundrySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Valid

Critical Logic Error in claimThrone() renders entire contract unusable and permanently locks all funds

tanug

Critical Logic Error in Game::claimThrone() renders entire contract unusable and permanently locks all funds

Description

  • The Game::claimThrone() function contains a fatal logic error in its access control check.

  • The require should use != instead of ==. Since currentKing is initialized to address(0) and msg.sender can never be address(0), this condition will always fail, making the function permanently uncallable.

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
);
}

Risk

Likelihood:

  • High: No one can ever claim the throne

  • declareWinner() can never be called (requires currentKing != address(0))

  • resetGame() can never be called (requires gameEnded = true from declareWinner())

Impact:

  • The contract becomes a fund sink with no recovery mechanism

  • Complete loss of functionality

  • Contract is DOA (dead on arrival)

Proof of Concept

A user with balance more than claimFee fails to call claimThone() and shows the incorrect error message saying - Game: You are already the king. No need to re-claim.

function test_claim_throne() external {
uint256 amountNeededToClainThrone = game.claimFee();
uint256 player1Balance = player1.balance;
assertGt(player1Balance, amountNeededToClainThrone);
vm.startPrank(player1);
vm.expectRevert("Game: You are already the king. No need to re-claim.");
game.claimThrone{value: INITIAL_CLAIM_FEE}();
vm.stopPrank();
}

Recommended Mitigation

Change == to != in the require statement of access control

- 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.");
Updates

Appeal created

inallhonesty Lead Judge about 2 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Game::claimThrone `msg.sender == currentKing` check is busted

Support

FAQs

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