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

Incorrect check implementation prevents users from entering the game

0xbyteknight

Root + Impact

Description

  • The game design assumes that a player is a current king, shouldn't be able to claim throne while still being a king.

  • The check, in the claimThrone function, which should make sure that current king won't claim the throne again, is incorrectly implemented and prevents anyone from entering the game.

require(
@> msg.sender == currentKing,
"Game: You are already the king. No need to re-claim."
);

Risk

Likelihood:

  • The issue will occur everytime when somebody will try to call the claimThrone function because it will check msg.sender against the 0 address.

Impact:

  • Incorrectly implemented check makes smart contract unusable right after deployment.

  • Unaware players will lose gas used to submit tx.

Proof of Concept

The following test should be added to the Game.t.sol file. This test shows demonstrates scenarios where a player tries to enter the game and tx reverts with error.

function testClaimThrone_RevertOnThroneClaiming() public {
// Deploy a new game
vm.prank(deployer);
game = new Game(
INITIAL_CLAIM_FEE,
GRACE_PERIOD,
FEE_INCREASE_PERCENTAGE,
PLATFORM_FEE_PERCENTAGE
);
​
// Check who is the current king
console2.logAddress(game.currentKing());
​
// New player tries to enter the game
vm.prank(player1);
vm.expectRevert("Game: You are already the king. No need to re-claim.");
game.claimThrone{value: INITIAL_CLAIM_FEE}();
}

Recommended Mitigation

The check must be update to make sure that msg.sender isn't the current king instead of comparing msg.sender with the currentKing value.

​
require(
- msg.sender == currentKing,
+ msg.sender != currentKing,
"Game: You are already the king. No need to re-claim."
);
Updates

Appeal created

inallhonesty Lead Judge 4 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.

Give us feedback!