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

Reset Game resets custom set grace period - Every round Owner has to set the custom grace period manually if custom value desired

rjl

Root + Impact

Description

  • In the current Game contract behaviour, setting a grace period to a new value by the Owner with updateGracePeriod will only persist until resetGame is called. This many not be desirable - It is not clear if this is a bug or a feature. This ideally should be queried with the game designer if this effect is intentional.

  • Motivation for why this might be a bug: Other custom set game parameters, that are set by the Owner any time after the constructor, persist between rounds, this seems logical, and seems easiest for a game Owner to change paramaters and for those effects to persist between rounds until a different value is set. Setting initialClaimFee, feeIncreasePercentage, and platformFeePercentage keep their effects between game rounds when called by the admin functions that set these functions updateClaimFeeParameters and updatePlatformFeePercentage. It seems logical that setting a new gracePeriod with a similar named function updateGracePeriod should behave in the same way.

  • If we want a new custom set gracePeriod to persist and not default back to the value of initialGracePeriod, between rounds, we could remove the line of code that resets the gracePeriod to initialGracePeriod in the resetGame function.


// Root cause in the codebase with marks to highlight the relevant section
​
function resetGame() external onlyOwner gameEndedOnly {
currentKing = address(0);
lastClaimTime = block.timestamp;
pot = 0;
claimFee = initialClaimFee;
@> gracePeriod = initialGracePeriod;
gameEnded = false;
gameRound = gameRound + 1;
​
emit GameReset(gameRound, block.timestamp);
}
​
​

Risk

Likelihood:

  • The potential issue of gracePeriod being reset to an intitial value would occur before every new game, when resetGame is called.


Impact:

  • Game Owner would need to change the gracePeriod every single game, if a different gracePeriod was desired than the value set initially when the Game constructor was called and set to initialGracePeriod.

  • Impact: Game owner to needs to call updateGracePeriod if a custom gracePeriod is wanted for each game that differs from the value of initialGracePeriod, this doesn't seem intutive, and its more work for the Game Owner than if gracePeriod simply remained at its last set value.

Proof of Concept

The grace period is being reset to the value of initialGracePeriod, this may not be desirable behaviour as any change set by the Owner to this value using the updateGracePeriod function will be reset back to the initial Value set in the constructor.

function resetGame() external onlyOwner gameEndedOnly {
currentKing = address(0);
lastClaimTime = block.timestamp;
pot = 0;
claimFee = initialClaimFee;
gracePeriod = initialGracePeriod;
gameEnded = false;
gameRound = gameRound + 1;
​
emit GameReset(gameRound, block.timestamp);
}

Recommended Mitigation

Remove the line of code that resets the gracePeriod in the resetGame function. This ensures that the custom new gracePeriod that is set by the Game owner using updateGracePeriod persists between games.

function resetGame() external onlyOwner gameEndedOnly {
currentKing = address(0);
lastClaimTime = block.timestamp;
pot = 0;
claimFee = initialClaimFee;
- gracePeriod = initialGracePeriod;
gameEnded = false;
gameRound = gameRound + 1;
​
emit GameReset(gameRound, block.timestamp);
}
Updates

Appeal created

inallhonesty Lead Judge 4 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!