Last Man Standing

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

The game can't be ended if no one calls `declareWinner()`.

Description

First, player1 claims the throne, and if no one calls declareWinner(), the game will not end. This leads to player2 being able to call claimThrone() even a whole day after the gracePeriod finished.

Proof of Concept

But before running the test, you need to correct this issue in claimThrone():

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

Then, add one of these tests to Game.t.sol:
When someone tries to claim the throne two days after the game starts.

This test will pass because no one called declareWinner():

function testclaimThrone2() public {
vm.startPrank(player1);
game.claimThrone{value: 0.1 ether}();
vm.stopPrank();
vm.warp(block.timestamp + 172800); // 2 days
vm.startPrank(player2);
game.claimThrone{value: 0.11 ether}(); // Still succeeds
vm.stopPrank();
}

But this test will fail and throw "Game has already ended. Reset to play again."
because someone called declareWinner():

function testclaimThrone2() public {
vm.startPrank(player1);
game.claimThrone{value: 0.1 ether}();
vm.stopPrank();
vm.warp(block.timestamp + 172800); // 2 days
vm.startPrank(player2);
game.declareWinner();
game.claimThrone{value: 0.11 ether}(); // Fails
vm.stopPrank();
}

run:
forge test --match-test testclaimThrone2 -vv

Recommended Mitigation

You can update the gameEnded flag inside claimThrone(),
or check if the gracePeriod has already ended or not inside claimThrone().

Updates

Appeal created

inallhonesty Lead Judge about 1 month ago
Submission Judgement Published
Validated
Assigned finding tags:

Game::claimThrone can still be called regardless of the grace period

Support

FAQs

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