Last Man Standing
First Flight #45
Beginner FriendlyFoundrySolidity
100 EXP
View results
Previous Next
Submission Details
Impact: medium
Likelihood: low
Invalid

`updateGracePeriod` lacks protections for active players

soupy

Summary

The updateGracePeriod function currently validates only that _newGracePeriod is greater than zero. However, it lacks additional safeguards to ensure that changes to the grace period do not negatively impact fairness for players who have already participated or are in the process of claiming thrones.

Description

The updateGracePeriod function is restricted to the contract owner, ensuring only authorized access. However, it performs minimal validation, only checking that the new grace period value is greater than zero. This lack of comprehensive checks gives the owner significant freedom to set a new grace period that may disadvantage the currentKing. Such a change could unfairly prevent the currentKing from claiming the cumulative pot they might have otherwise won if the original grace period remained unchanged.

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

Risk

Likelihood:

  • While the function is restricted to the contract owner, the absence of strict constraints on how the grace period can be changed introduces a risk of misuse, whether intentional or accidental. If the owner does not fully consider the game’s state, they could update the grace period in a way that unfairly impacts active players. This risk increases if the contract is not governed by a transparent and timely pre-alert communication prior to any changes.

Impact:

  • Changing the grace period arbitrarily can significantly alter game dynamics, particularly disadvantaging the currentKing who may be close to winning the cumulative pot. Such changes can undermine player trust, create perceptions of unfairness, and potentially result in financial losses for affected players. This could also lead to reputational damage for the project and reduce user participation.

Proof of Concept

In test/Game.t.sol, add the following test:

function test_audit_updateGracePeriodAfffectsCurrentKing() public {
vm.prank(player1);
game.claimThrone{value: 1 ether}();
​
vm.prank(player2);
game.claimThrone{value: INITIAL_CLAIM_FEE * 110 / 100}();
​
// fast forward to 30 minutes before the game ends
vm.warp(game.lastClaimTime() + GRACE_PERIOD - 30 minutes);
uint256 remainingTime = game.getRemainingTime();
console2.log("remaining_time_b4_game_end: ", remainingTime);
console2.log("current_king: ", game.currentKing());
console2.log("pot_value: ", game.pot());
// during this time, owner update the grade period before the game ends
vm.prank(deployer);
game.updateGracePeriod(2 days);
uint256 newRemainingTime = game.getRemainingTime();
​
console2.log("remaining_time_aft_owner_update: ", newRemainingTime);
console2.log("current_king: ", game.currentKing());
console2.log("pot_value: ", game.pot());
​
// the passing of the assertion test shows that the remaining time has increased
// which means the game end time is pushed further
// player2 who could have won the pot in 30min when the game ends has now in disadvantage state that the pot could be won by someone else as the game duration is prolonged
assertGt(newRemainingTime, remainingTime);
}

In terminal, run forge test --match-test test_audit_updateGracePeriodAfffectsCurrentKing -vvv will generate the following results:

$ forge test --match-test test_audit_updateGracePeriodAfffectsCurrentKing -vvv
...
​
Ran 1 test for test/Game.t.sol:GameTest
[PASS] test_audit_updateGracePeriodAfffectsCurrentKing() (gas: 221763)
Logs:
remaining_time_b4_game_end: 1800
current_king: 0xEb0A3b7B96C1883858292F0039161abD287E3324
pot_value: 1054500000000000000
​
remaining_time_aft_owner_update: 88200
current_king: 0xEb0A3b7B96C1883858292F0039161abD287E3324
pot_value: 1054500000000000000
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.16ms (431.92µs CPU time)
​
Ran 1 test suite in 118.98ms (1.16ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Recommended Mitigation

Consider to Implement additional safeguards in the updateGracePeriod function to prevent arbitrary or unfair changes that could impact active game play such as:

  1. add delay timeframe for the new grade period to take effect

  2. implement pot value boundaries to prevent further update of grade period to ensure fairness to the current king

  3. delegate the authority to a decentralized governance mechanism

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.