Rock Paper Scissors
First Flight #38
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Stale revealDeadline Enables Premature Timeout and Game Exploitation

ddeenny

Summary

The RockPaperScissors::_determineWinner function handles the conclusion of each turn and resets key state variables for the next round. However, it fails to clear or reset the revealDeadline field between turns. As a result, a stale revealDeadline from a previous round may incorrectly trigger timeouts in future rounds, leading to unintended game cancellations or premature wins.

Vulnerability Details

function _determineWinner(uint256 _gameId) internal {
...
if (game.currentTurn < game.totalTurns) {
game.currentTurn++;
game.commitA = bytes32(0);
game.commitB = bytes32(0);
game.moveA = Move.None;
game.moveB = Move.None;
game.state = GameState.Committed;
// @audit-issue revealDeadline is not cleared or updated for the new turn
@> // game.revealDeadline remains from previous round
}
...
}

Issue Explanation

The revealDeadline is set only after both players commit moves, but never cleared between rounds:

  1. Stale Deadline Persists
    If a round ends just before the deadline, the next round starts with a leftover revealDeadline that may soon expire.

  2. Incorrect Timeout Claims
    A malicious player could wait for the new round to begin, then immediately call timeoutReveal() using the expired revealDeadline, wrongly forcing a win or a refund.

  3. State Corruption Risk
    Downstream logic that depends on revealDeadline being valid for the current round may behave unpredictably if it's stale.

Impact

  • Game Manipulation: Players can exploit stale deadlines to claim unfair wins.

  • Denial of Service: Honest players may find their games cancelled or forfeited without fault.

  • Loss of Trust: Users may abandon the game if outcomes appear inconsistent or unfair.

Tools Used

  • Manual Code Review

Recommendations

Reset revealDeadline to 0 at the start of every new round in _determineWinner, to prevent stale values from being reused:

if (game.currentTurn < game.totalTurns) {
game.currentTurn++;
game.commitA = bytes32(0);
game.commitB = bytes32(0);
game.moveA = Move.None;
game.moveB = Move.None;
game.revealDeadline = 0; // @fix Reset deadline between turns
game.state = GameState.Committed;
}
Updates

Appeal created

m3dython Lead Judge 2 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Invalid TimeoutReveal Logic Error

timeoutReveal function incorrectly allows execution and game cancellation even when only one player has committed

m3dython Lead Judge 2 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Invalid TimeoutReveal Logic Error

timeoutReveal function incorrectly allows execution and game cancellation even when only one player has committed

Support

FAQs

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