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

Cannot call claimThrone as user

neomartis

Cannot call claimThrone as user

Description

  • claimThrone should be able to be called by any user, EXCEPT the current king

  • The current behaviour is the inverse, where ONLY the current king can call claimThrone

function claimThrone() external payable gameNotEnded nonReentrant {
require(msg.value >= claimFee, "Game: Insufficient ETH sent to claim the throne.");
// This require statement incorrectly checks that the caller is the current king
@> require(msg.sender == currentKing, "Game: You are already the king. No need to re-claim.");

Risk

Likelihood: High

  • This happens for every user every time they call claimThrone

  • Initially the currentKing is the zero-address, meaning no one can call claimThrone

Impact: High

  • This function is a core functionality of the contract

  • Without this function, the whole game becomes unusable, resulting in no-one being able to claim the throne and become a winner

  • The only way to fix this after deployment is to redeploy the contract, which is a major inconvenience and a waste of gas

Proof of Concept

To validate the issue, we can run the following test (with the default setup as per Game.t.sol)

function testIssueClaimThroneAsUserResultsInRevert() public {
console2.log("player1", player1);
console2.log("Initial currentKing()", game.currentKing());
vm.prank(player1);
// ISSUE: The game.claimThrone() will revert, which is unintended behaviour
vm.expectRevert("Game: You are already the king. No need to re-claim.");
game.claimThrone{value: INITIAL_CLAIM_FEE}();
console2.log("After claimThrone currentKing()", game.currentKing());
}

logs

Logs:
player1 0x7026B763CBE7d4E72049EA67E89326432a50ef84
Initial currentKing() 0x0000000000000000000000000000000000000000
After claimThrone currentKing() 0x0000000000000000000000000000000000000000

As shown in the logs, the throne is never claimed, and the function is reverted with "Game: You are already the king. No need to re-claim"

Recommended Mitigation

The issue involves a simple fix by reverting the == check in the require statement to !=. This will correctly check to ensure the caller is not the current king.

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