Last Man Standing

First Flight #45
Beginner FriendlyFoundrySolidity
100 EXP
View results
Submission Details
Impact: medium
Likelihood: medium
Invalid

Deployer can update `gracePeriod` mid game.

Deployer can update gracePeriod mid game.

Description:

Parameters set for a game should not be altered during the game. The deployer can update gracePeriod mid-game. This could potentially cause confusion and disruption of the protocol. A malicious deployer could use this to manipulate the outcome of a game, allowing the current king to gain an advantage.

Impact: Medium

  • While no funds are directly at risk, many players could be put at a disadvantage, especially if the new grace period is set to a short amount.

Likelihood: Medium

  • There are no checks on being able to change the gracePeriod parameter only when the game is not active.

Proof of Concept:

  1. Create a game with a one-day grace period

  2. VM.warp 1 hour

  3. Deployer updates grace period to 1 hour

  4. The game is now over

Proof of Code:

Paste the below code into Game.t.sol.

function test_CanGracePeriodUpdateMidGame() public {
// Current game time remaining
uint256 timeRemaining = game.getRemainingTime();
console2.log("Time remaining in the game:", timeRemaining);
// Current grade period
console2.log("Current grace period:", game.gracePeriod());
// Let one hour of gametime pass
console2.log("Advancing time by 1 hour...");
vm.warp(block.timestamp + 1 hours);
// Deployer updates the grace period mid game
vm.startPrank(deployer);
uint256 newGracePeriod = 1 hours; // Update to 1 hour
console2.log("Updating grace period to:", newGracePeriod);
game.updateGracePeriod(newGracePeriod);
vm.stopPrank();
// Verify the game time remaining after updating grace period - no time left
uint256 timeRemaining2 = game.getRemainingTime();
console2.log("Time remaining in the game:", timeRemaining2);
// Verify the new grace period
assertEq(game.gracePeriod(), newGracePeriod, "Grace period should be updated to 2 days");
}

Here is the output after running:

"forge test --mt test_CanGracePeriodUpdateMidGame -vv"

Logs:
Time remaining in the game: 86400
Current grace period: 86400
Advancing time by 1 hour...
Updating grace period to: 3600
Time remaining in the game: 0

Recommended Mitigation:

I would recommend adding the gameEndedOnly modifier to game::updateGracePeriod. This would prevent the deployer from updating the grace period while a game is ongoing!

Here is how to make the change:

+ function updateGracePeriod(uint256 _newGracePeriod) external onlyOwner gameEndedOnly {
- function updateGracePeriod(uint256 _newGracePeriod) external onlyOwner {
require(_newGracePeriod > 0, "Game: New grace period must be greater than zero.");
gracePeriod = _newGracePeriod;
emit GracePeriodUpdated(_newGracePeriod);
}
Updates

Appeal created

inallhonesty Lead Judge about 2 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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