Rock Paper Scissors

First Flight #38
Beginner FriendlySolidity
100 EXP
View results
Submission Details
Severity: medium
Invalid

Unrestricted Game Creation Allows Resource Exhaustion

Summary

The RockPaperScissors contract allows players to create an unlimited number of games simultaneously without any restrictions. A malicious player can exploit this vulnerability to create numerous games, potentially causing resource exhaustion, degrading user experience, and temporarily locking funds in abandoned games.

Vulnerability Details

In theRockPaperScissors::createGameWithEth and RockPaperScissors::createGameWithToken functions, there is no mechanism to limit the number of active games a single player can create:

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(_timeoutInterval >= 5 minutes, "Timeout must be at least 5 minutes");
uint256 gameId = gameCounter++;
// No check for how many active games the player already has
Game storage game = games[gameId];
game.playerA = msg.sender;
game.bet = msg.value;
// ...
}

Impact

This vulnerability has several negative impacts on the system:

  1. Storage Bloat: Malicious users can create numerous games, increasing the contract's storage requirements and potentially increasing gas costs for other operations.

  2. Funds Locking: Players joining these games will have their funds temporarily locked until timeout resolution, causing frustration and degrading user experience.

  3. Game Manipulation: A player could create multiple games with different parameters to increase their chances of favorable matchups or game conditions.

  4. Abandoned Games: Players can strategically abandon games they're losing, focusing only on games with favorable outcomes, which reduces the integrity of the game system.

  5. Denial of Service: At scale, this could lead to a form of DoS attack by making legitimate game discovery more difficult among numerous abandoned or inactive games.

  6. Implementing a reasonable limit on the number of active games per player would mitigate this vulnerability while maintaining the intended functionality of the contract

Tools Used

Manual Review

Recommendations

To address the unrestricted game creation vulnerability, I recommend implementing the following changes:

+ mapping(address => uint256) public playerActiveGameCount;
+ uint256 public constant MAX_ACTIVE_GAMES_PER_PLAYER = 3;
// Modify the game creation functions
function createGameWithEth(uint256 _totalTurns, uint256 _timeoutInterval) external payable returns (uint256) {
+ require(playerActiveGameCount[msg.sender] < MAX_ACTIVE_GAMES_PER_PLAYER, "Too many active games");
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(_timeoutInterval >= 5 minutes, "Timeout must be at least 5 minutes");
uint256 gameId = gameCounter++;
Game storage game = games[gameId];
game.playerA = msg.sender;
game.bet = msg.value;
game.timeoutInterval = _timeoutInterval;
game.creationTime = block.timestamp;
game.joinDeadline = block.timestamp + joinTimeout;
game.totalTurns = _totalTurns;
game.currentTurn = 1;
game.state = GameState.Created;
// Increment the player's active game count
+ playerActiveGameCount[msg.sender]++;
emit GameCreated(gameId, msg.sender, msg.value, _totalTurns);
return gameId;
}
+ // Apply similar changes to createGameWithToken()
// Update game completion functions to decrement the counter
function _finishGame(uint256 _gameId, address _winner) internal {
Game storage game = games[_gameId];
// Existing code...
+ // Decrement active game counters
+ if (game.state != GameState.Finished && game.state != GameState.Cancelled) {
+ playerActiveGameCount[game.playerA]--;
+ if (game.playerB != address(0)) {
+ playerActiveGameCount[game.playerB]--;
+ }
+ }
game.state = GameState.Finished;
// Rest of the existing function...
}
+ // Similar updates to _cancelGame() and other functions that end games
+ // Add a view function to check a player's active game count
+ function getPlayerActiveGameCount(address player) external view returns (uint256) {
+ return playerActiveGameCount[player];
+ }

This implementation:

  1. Tracks the number of active games per player

  2. Enforces a reasonable limit (3 in this example, but can be adjusted)

  3. Properly increments and decrements counters as games are created and completed

  4. Provides a view function to check active game counts

The value for MAX_ACTIVE_GAMES_PER_PLAYER should be carefully chosen to balance user flexibility with system protection. A value between 3-5 is reasonable for most use cases, preventing abuse while allowing legitimate players to have multiple active games.
Additionally, consider implementing a game inactivity timeout mechanism to automatically cancel games that have been in the Created state for too long without player B joining, which would further mitigate the impact of abandoned games.

Updates

Appeal created

m3dython Lead Judge about 1 month ago
Submission Judgement Published
Invalidated
Reason: Design choice
m3dython Lead Judge about 1 month ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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