Rock Paper Scissors

Description:

The timeoutJoin() function is intended to allow a player to cancel a game if the joining deadline has passed and no second player has joined. However, the function does not validate that the provided _gameId corresponds to an actual game that was initiated via the createGameWithEth() or similar creation functions.
When timeoutJoin() is called with a _gameId that has not been used as an index in the games mapping (i.e., _gameId is greater than or equal to gameCounter), Solidity's default behavior for mapping access provides a zero-initialized Game struct at that storage location. The default values for the Game struct members are address(0) for addresses, 0 for integers, bytes32(0) for bytes32, and the first enum value (which is GameState.Created for GameState).
The require conditions within timeoutJoin() evaluate as follows for a default-initialized Game struct:

Passes, as the default state is GameState.Created.

Passes, as the default joinDeadline is 0 and block.timestamp is almost always greater than zero.

Passes, as the default playerB is address(0).

Since all checks pass, the _cancelGame function is called with the uncreated _gameId. _cancelGame then updates the state of this default struct in storage to GameState.Cancelled, effectively writing a new entry into the games mapping.

Impact:

An attacker can repeatedly call the timeoutJoin() function with unique, uncreated _gameId values. Each successful call will create a new entry in the public games mapping with a GameState.Cancelled. This leads to state bloat, where the contract's storage size increases unnecessarily with meaningless, cancelled game entries.
State bloat increases the cost of contract deployment and can increase the gas costs for any operations that interact with the storage containing the games mapping. While mapping lookups generally have a consistent gas cost regardless of the number of entries, operations like contract upgrades or potentially future state rent implementations could be negatively impacted by excessive storage size. More directly, it allows an attacker to grief the contract by forcing it to consume more network resources (storage gas) at the expense of legitimate users who eventually bear these costs.

Proof of Concept:

• An attacker selects a _gameId that is known not to correspond to an active or previously used game (e.g., a large number outside the range of gameCounter).

• The attacker calls timeoutJoin(chosen_gameId).

• Inside timeoutJoin, games[chosen_gameId] is accessed. Since it's a new key, a default-initialized Game struct is provided from storage.

• The require checks pass because the default game.state is Created, game.joinDeadline is 0, and game.playerB is address(0).

• _cancelGame(chosen_gameId) is called.

• Inside _cancelGame, the state of games[chosen_gameId] is set to GameState.Cancelled. The refund and token logic is skipped as the default game.bet is 0, and game.playerA and game.playerB are address(0).

• A new entry for chosen_gameId is now permanently stored in the games mapping with a Cancelled state. The attacker can repeat this with different IDs to bloat the storage.

Recommended Mitigation:

The timeoutJoin function should verify that the _gameId corresponds to a game that was previously created. This can be done by checking if the _gameId is less than the current gameCounter. If _gameId is greater than or equal to gameCounter, it means the ID has not been assigned to a created game, and the function call should be reverted.
Add a require check at the beginning of the timeoutJoin function:

Relevant Code:

• RockPaperScissors.sol: timeoutJoin function, _cancelGame function, games mapping , Game struct, gameCounter state variable