Last Man Standing

First Flight #45
Beginner FriendlyFoundrySolidity
100 EXP
View results
Submission Details
Severity: medium
Valid

[H-1] Logic error: ``msg.sender == currentKing`` blocks throne claiming

Logic error: msg.sender == currentKing blocks throne claiming

Description

The claimThrone function is designed to allow players to claim the throne by sending a sufficient amount of ETH (claimFee). When successful, the player becomes the new king.

However, the following condition incorrectly prevents anyone other than the current king from using the function:

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

Risk

Likelihood:

  • Occurs always, after the initial king has been established, because no other player appart from the initial one can claim the throne anymore.

Impact:

  • Game cannot progress, defeating its purpose.

Proof of Concept

To recreate this vulnerability write the following test in Game.t.sol:

function testPlayerClaimsThroneAndFails() public {
vm.prank(player1);
console2.log("Initial king of the hill `address(0)`", game.currentKing());
console2.log("Expected new king of the hill", player1);
vm.expectRevert("Game: You are already the king. No need to re-claim.");
game.claimThrone{value: INITIAL_CLAIM_FEE}();
console2.log("King after player1 -> claimThrone()", game.currentKing());
}

As shown with this text, player1 is unable to claim the throne via claimThrone since the function is reverting due to msg.sender == currentKing requirement

Recommended Mitigation

A good mitigation is rewriting the require logic to make sure the player claiming the throne is not the current king.

- 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 1 month 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.