Rock Paper Scissors

Summary

The Rock Paper Scissors game smart contract does not allow players to update their committed moves, even if the reveal phase hasn't begun. This creates a poor user experience where players cannot correct mistakes in their commitments, potentially leading to unintended game outcomes, timeouts, and lost funds.

Vulnerability Details

In the Rock Paper Scissors game, players follow a commit-reveal pattern where they first commit a hashed move (combining their move with a salt value) and later reveal the actual move with the salt. The issue arises in the commitMove function (lines 317-351), which doesn't allow players to update their commitments once made:

The issue specifically lies in the requirements require(game.commitA == bytes32(0), "Already committed") and require(game.commitB == bytes32(0), "Already committed"), which prevent players from updating their commitments once made, even if:

1. The reveal phase hasn't started yet

2. No one has revealed their move yet

3. The player realized they made a mistake with their salt or move

This creates a situation where a player who makes a mistake in their commitment:

• May not be able to provide a valid reveal

• Could be penalized by timing out during the reveal phase

• Could lose their wagered funds unfairly

Impact

This vulnerability has several impacts:

1. Player Experience Degradation: Players cannot correct legitimate mistakes in their commitments.

2. Forced Timeouts: Players who commit incorrectly will be forced to time out during the reveal phase.

3. Unfair Outcomes: A player who knows they made a mistake must either forfeit or rely on the other player also failing to reveal.

4. Increased Support Burden: Game operators would likely face increased support requests from frustrated players who made commitment mistakes.

Tools Used

• Manual code review

• User flow analysis

• Game theory analysis

Recommendations

1. Allow Commitment Updates:
   Modify the commitMove function to allow players to update their commitments before the reveal phase begins:

   function commitMove(uint256 _gameId, bytes32 _commitHash) external {

   Game storage game = games[_gameId];

   ​

   require(msg.sender == game.playerA || msg.sender == game.playerB, "Not a player in this game");

   require(game.state == GameState.Created || game.state == GameState.Committed, "Game not in commit phase");

   // Check that reveal phase hasn't started yet

   require(game.moveA == Move.None && game.moveB == Move.None, "Reveal phase has begun");

   ​

   // ... other existing checks ...

   ​

   if (msg.sender == game.playerA) {

   // Remove the requirement that commitA must be bytes32(0)

   game.commitA = _commitHash;

   } else {

   // Remove the requirement that commitB must be bytes32(0)

   game.commitB = _commitHash;

   }

   ​

   // ... existing event emission and deadline setting ...

   }

2. Add Explicit Recommit Function:
   Alternatively, add a dedicated function for updating commitments:

   function updateCommit(uint256 _gameId, bytes32 _newCommitHash) external {

   Game storage game = games[_gameId];

   require(msg.sender == game.playerA || msg.sender == game.playerB, "Not a player in this game");

   require(game.state == GameState.Committed, "Game not in commit phase");

   // Ensure reveal phase hasn't started

   require(game.moveA == Move.None && game.moveB == Move.None, "Reveal phase has begun");

   // Update the commitment

   if (msg.sender == game.playerA) {

   game.commitA = _newCommitHash;

   } else {

   game.commitB = _newCommitHash;

   }

   emit MoveCommitted(_gameId, msg.sender, game.currentTurn);

   }