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

Stuck Games During Commit Phase Leads to Loss of Fund

roccomania

Summary
Games can become permanently stuck if one player commits their move but the second player never does. This occurs because the revealDeadline is only set after both players commit, leaving no timeout mechanism for the commit phase itself once the game state has transitioned to Committed.

Vulnerability Details
The commitMove function handles the transition from Created to Committed state and the setting of the revealDeadline:

When the first player commits (and Player B has joined), the state is potentially changed:

if (game.currentTurn == 1 && game.commitA == bytes32(0) && game.commitB == bytes32(0)) {
// First turn, first commits
require(game.playerB != address(0), "Waiting for player B to join");
game.state = GameState.Committed; // State changes here!
}
​

However, the revealDeadline, which is crucial for the timeoutReveal function, is only set later in the same function, after checking if both players have committed:

// If both players have committed, set the reveal deadline
if (game.commitA != bytes32(0) && game.commitB != bytes32(0)) {
game.revealDeadline = block.timestamp + game.timeoutInterval; // Deadline only set here
}
​

If Player A commits (changing the state to Committed), but Player B never calls commitMove, the condition game.commitA != bytes32(0) && game.commitB != bytes32(0) will never be true. Consequently, game.revealDeadline remains 0.

  • timeoutReveal cannot be called because it requires block.timestamp > game.revealDeadline, which fails if revealDeadline is 0 or hasn't passed.

  • cancelGame cannot be called because it requires game.state == GameState.Created, but the state is now Committed.

  • timeoutJoin cannot be called because Player B has joined (otherwise, the state wouldn't have changed to Committed on the first commit).

The game is therefore stuck in the Committed state with no path forward to resolution or cancellation.

Impact

  • Permanent Game Stall: The game cannot proceed or be cancelled, becoming permanently unusable.

  • Locked Funds/Tokens: The ETH or WinningTokens deposited by both Player A and Player B for the game remain locked in the contract indefinitely.

Tools Used
Manual code review.

Recommendations
Introduce a timeout specifically for the commit phase after the game state becomes Committed.

  • Add commitDeadline to Game struct:

struct Game {
// ... existing fields ...
uint256 revealDeadline; // Deadline for revealing moves
+ uint256 commitDeadline; // Deadline for second player to commit
// ... rest of fields ...
}
​

  • Set commitDeadline when state becomes Committed: Modify commitMove to set this deadline when the first player commits and the state changes. Use a reasonable duration (e.g., similar to joinTimeout or timeoutInterval).

// In commitMove:
if (game.currentTurn == 1 && game.commitA == bytes32(0) && game.commitB == bytes32(0)) {
// First turn, first commits
require(game.playerB != address(0), "Waiting for player B to join");
game.state = GameState.Committed;
+ // Set a deadline for the second player to commit
+ game.commitDeadline = block.timestamp + game.timeoutInterval; // Or another suitable duration
}
​

  • Create timeoutCommit function: Add a new external function allowing either player (or potentially anyone) to cancel the game if the commitDeadline passes and only one player has committed.

/**
* @notice Cancel game if the second player fails to commit in time
* @param _gameId ID of the game
*/
function timeoutCommit(uint256 _gameId) external {
Game storage game = games[_gameId];
​
require(game.state == GameState.Committed, "Game not in commit phase");
require(block.timestamp > game.commitDeadline, "Commit deadline not passed yet");
require(
(game.commitA != bytes32(0) && game.commitB == bytes32(0)) ||
(game.commitA == bytes32(0) && game.commitB != bytes32(0)),
"Both players have committed or neither has" // Should ideally only trigger if one has committed
);
​
// Cancel the game and refund players
// Consider if the non-committing player should be penalized (e.g., forfeit)
// For simplicity, using _cancelGame for refund logic:
_cancelGame(_gameId);
}
​

  • Adjust commitMove checks: Ensure commitMove also checks against commitDeadline if applicable.

Updates

Appeal created

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

Game State Manipulation Preventing Opponent Commit

Attack allows a player to reveal their move for the next turn before the opponent commits

Support

FAQs

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