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

Missing state update in `Game::gameNotEnded` modifier allows claims after `Game::gracePeriod` has expired

codedesignz

Missing state update in Game::gameNotEnded modifier allows claims after Game::gracePeriod has expired

Description

  • The protocol enforces a grace period after which no new players should be allowed to participate in the current game round.

  • The Game::gameNotEnded modifier only checks whether gameEnded is true, but does not update the state based on the current block timestamp. If the Game::gracePeriod has already expired and gameEnded has not yet been updated (typically set during a call to Game::declareWinner), new players can still claim the throne. This oversight permits continued participation beyond the intended end of the game round.

modifier gameNotEnded() {
@> // @CD-audit: This check only verifies if `gameEnded` is true or false. It does not update the state based on expiration of `Game::gracePeriod`, leading to incorrect behavior.
require(!gameEnded, "Game: Game has already ended. Reset to play again.");
_;
}

Risk

Likelihood:

  • Any time a new player calls the Game::claimThrone function, they can successfully claim the throne even after the gracePeriod has expired, unless someone has explicitly called Game::declareWinner to update the gameEnded flag.

  • The Game::declareWinner function is a manual action. If not triggered promptly, it leaves the game open for unintended claims.

Impact:

  • Players can continue to claim the throne after the grace period has ended.

  • This breaks the core protocol logic that restricts claims to within the grace period, undermining fairness and the intended game mechanics.

Proof of Concept

The following test demonstrates that a new player can successfully claim the throne after the grace period has elapsed. Add this function to your existing test suite and execute it:

function testClaimThroneAfterGracePeriod() public {
vm.startPrank(player1);
game.claimThrone{value: 0.1 ether}();
vm.stopPrank;
vm.warp(block.timestamp + (GRACE_PERIOD + 1 seconds));
console2.log("Player2 address: ", player2);
vm.startPrank(player2);
game.claimThrone{value: game.claimFee()}();
vm.stopPrank;
​
console2.log("Current King: ", game.currentKing());
}

Recommended Mitigation

Update the Game::gameNotEnded modifier to also check and update the game state based on the current timestamp. This ensures the gameEnded flag is accurate at every relevant call site.

modifier gameNotEnded() {
+ setGameState();
require(!gameEnded, "Game: Game has already ended. Reset to play again.");
_;
}
​
+ function setGameState() private {
+ gameEnded = block.timestamp > lastClaimTime + gracePeriod;
+ }
Updates

Appeal created

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