Rock Paper Scissors

Summary

The RockPaperScissors.sol contract uses uint8 variables (scoreA, scoreB) within the Game struct to track player scores across multiple turns. A uint8 has a maximum value of 255. The game creation functions (createGameWithEth, createGameWithToken) allow setting the totalTurns for a game without an upper limit related to this score capacity. If a game is configured with a high number of turns (e.g., > 255) and one player achieves 256 or more wins, the score increment operation (game.scoreA++ or game.scoreB++) within the _determineWinner function will trigger an arithmetic overflow. Due to Solidity version 0.8.x's default checked arithmetic, this overflow will cause the transaction to revert, leading to a Denial of Service (DoS) for the game's completion and potentially locking the staked funds.

Vulnerability Details

1. Data Type Limitation: The scoreA and scoreB members of the Game struct are defined as uint8, restricting their maximum value to 255.

   struct Game {

   // ...

   uint8 scoreA; // Max value 255

   uint8 scoreB; // Max value 255

   // ...

   }

2. Score Increment: The _determineWinner function increments the winner's score using the ++ operator.

   function _determineWinner(uint256 _gameId) internal {

   // ... logic to determine winner ...

   if (/* Player A wins */) {

   game.scoreA++; // Potential overflow if scoreA is 255

   // ...

   } else {

   // Player B wins

   game.scoreB++; // Potential overflow if scoreB is 255

   // ...

   }

   // ...

   }

3. Checked Arithmetic: The contract uses pragma solidity ^0.8.13;. Versions 0.8.0 and higher have built-in overflow and underflow checks for arithmetic operations. An attempt to increment a uint8 variable from 255 to 256 will result in a revert.

4. Lack of Input Validation: The createGameWithEth and createGameWithToken functions validate that _totalTurns is positive and odd but do not impose an upper limit based on the uint8 score capacity. A user can create a game with, for example, 301 turns.

5. Exploitation Scenario: If a game is created with totalTurns = 301, and Player A wins 256 turns before the game concludes, the next time Player A wins a turn, _determineWinner will attempt game.scoreA++ on the value 255. This operation will revert due to overflow.

Proof Of Concept

The provided test case demonstrates this vulnerability

Impact

• Denial of Service (DoS): Games configured with a number of turns potentially allowing scores > 255 cannot be completed if a player reaches 256 wins. The game becomes stuck, unable to process the final turns or determine a winner.

• Locked Funds: When the transaction reverts during the score increment (likely during the revealMove call of the second player), the game state remains unresolved (Committed or partially revealed). The payout functions (_finishGame, _handleTie) are never reached, resulting in the permanent locking of staked ETH or Tokens within the contract for that game.

• Game Design Limitation: The use of uint8 imposes an implicit, potentially unexpected limit on the maximum practical number of turns or winning rounds per player, regardless of the uint256 type used for totalTurns.

Tools Used

• Manual Code Review

• Foundry/Forge (for Test Execution, PoC verification, and vm.store)

Recommendations

To mitigate this vulnerability, the data type for scores should be adjusted, or input validation should be added to prevent configurations that could lead to overflow.

1. Increase Score Variable Size (Recommended): Change the data type of scoreA and scoreB in the Game struct from uint8 to a larger integer type, such as uint16.

   struct Game {

   // ...

   - uint8 scoreA; // Score for player A

   - uint8 scoreB; // Score for player B

   + uint16 scoreA; // Score for player A (Max 65535)

   + uint16 scoreB; // Score for player B (Max 65535)

   // ...

   }

   A uint16 allows for up to 65,535 wins per player, which is highly unlikely to be exceeded in any practical Rock Paper Scissors game scenario, effectively eliminating the overflow risk while still being reasonably gas-efficient.

2. Add Input Validation (Alternative): Alternatively, enforce a maximum limit on _totalTurns during game creation that ensures scores cannot exceed 255.

   function createGameWithEth(uint256 _totalTurns, uint256 _timeoutInterval) external payable returns (uint256) {

   require(msg.value >= minBet, "Bet amount too small");

   require(_totalTurns > 0, "Must have at least one turn");

   require(_totalTurns % 2 == 1, "Total turns must be odd");

   + require(_totalTurns <= 509, "Total turns exceeds score capacity"); // Max turns for uint8 score (255 * 2 + 1 = 511, but odd required)

   require(_timeoutInterval >= 5 minutes, "Timeout must be at least 5 minutes");

   // ...

   }

   // Similar check in createGameWithToken

   This approach is less flexible but prevents the overflow condition from ever being reachable.

Using uint16 for scores (Recommendation 1) is generally preferred as it provides ample headroom for game length without imposing potentially arbitrary limits during game creation.