Summary

The RockPaperScissors contract defines a GameState.Revealed in its state machine, but this state is never used or assigned anywhere in the contract. This indicates an incomplete or abandoned state transition design. The absence of a proper Revealed phase results in weak state checks and ambiguous transitions, increasing the risk of unexpected behavior or exploit paths, especially in edge cases.

Vulnerability Details

The GameState enum is defined as:

However:

• No function ever sets game.state = GameState.Revealed.

• All logic (e.g., in revealMove, _determineWinner) proceeds directly from Committed to Finished, bypassing this potential intermediate state.

• The revealMove function does not require that both moves be revealed before proceeding to winner determination, nor does it transition to Revealed.

This creates ambiguity in the game flow and weakens assumptions about the state machine. It also limits the ability to enforce expected behavior through require(game.state == ...) checks, which would normally safeguard state integrity.

Impact

• Weak state validation: Without a Revealed state, some functions cannot precisely determine the current game phase.

• Process ambiguity: Makes it harder to reason about transitions between phases (e.g., whether both players have revealed yet).

• Harder maintenance or extension: Future developers may incorrectly assume Revealed is used, introducing logic bugs.

• Increased attack surface: Missing state guards make functions vulnerable to unexpected or illogical calls.

Tools Used

• Manual code review

• Analysis of state transitions

• Inspection of function guards and lack thereof

Recommendations

1. Properly integrate the Revealed state into the game lifecycle:

   • After both players have called revealMove, set:

     game.state = GameState.Revealed;

   • Then, in _determineWinner, require:

     require(game.state == GameState.Revealed, "Not ready to determine winner");

2. Use state checks more strictly in all functions:

   • Add require(game.state == ExpectedState) in all public and internal functions where appropriate.

   • For example, restrict revealMove to only be callable in Committed, and timeoutReveal in Committed or Revealed (depending on when the deadline was meant to expire).

3. Alternatively, if simplifying the state machine is preferred, remove Revealed entirely and clearly document the logic that handles post-commit and post-reveal phases without state transitions.