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

[High] Nobody can claim the throne in `Game::claimThrone` since function only allows access to king which is address(0)

iframuazzam09

Root + Impact

Description

In Game::claimthrone, players should be able to "claim the throne" and become king by entering the function and paying a suitable fee. Current King should not be able to access the function as they are already king. But the require statement in claimThrone is incorrect making it so that only the king can enter and players cannot. This desrupts the functionality. Since they currentKing starts as address(0), nobody can access the function except address(0) and this address cannot be changed..

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: High.

  • Reason 1 Happens whenever a player tries to claimThrone but they can't, even though they fulfill all the requirements

  • Reason 2

Impact: High

  • Impact 1 Nobody can claim the throne which severely disrupts functionality of the protocol.

  • Impact 2 Address(0) will always remain king and they cannot be changed

Proof of Concept

There are two tests in the Proof of Code. First Test will prove that a normal player cannot enter the claim throne. While fuzz testing, No address could enter claim throne except address(0). In the Second, It will prove that address(0) can access the function but it is only address(0) and no other address.

//This function tests that a normal player cannot enter claimThrone even though they are not king
function test_NobodyCanClaimThrone()public {
vm.startPrank(player1); //when player tried to enter, the function call reverted saying that he is already king while the current king is address(0).
vm.expectRevert("Game: You are already the king. No need to re-claim.");
game.claimThrone{value: 0.1 ether}();
assertEq(game.currentKing(), address(0)); //This assertion tells that address(0) is king and not player1
}
​
// This function tests that address(0) can access claimThrone function while any other player could not
function test_ZeroAddressCanClaimThrone() public {
address player0 = address(0);
vm.deal(player0, 10 ether);
vm.prank(player0);
game.claimThrone{value: 0.1 ether}(); //address(0) can access the function while other players cannot.
assertEq(address(player0).balance, 9.9 ether);
}

Recommended Mitigation

Try correcting the require statement in Game::claimThrone so that it completes its intended purposse of making sure that players trying to claim the throne are not already king instead og checking whether players are kings and only k=giving them access.

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