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

Give us feedback!