Rock Paper Scissors

First Flight #38
Beginner FriendlySolidity
100 EXP
View results
Submission Details
Severity: high
Valid

Player Overwrite Vulnerability in Game Join Mechanism

Smart Contract Audit Report: Rock Paper Scissors Game

Summary

The joinGameWithEth function allows multiple players to join the same game as "Player B", overwriting the previous Player. This results in stolen game slots and permanently locked funds for the overwritten players, as there is no mechanism to withdraw their funds once overwritten.

Vulnerability Details

The vulnerability exists in both joinGameWithEth and joinGameWithToken functions. When a second player joins a game, the contract correctly updates the playerB address but fails to update the game state from GameState.Created to another state which will disable joining the game once it has started and both players complete.

Since the game remains in the Created state after a player joins, other players can call the join functions again with the same game ID. This overwrites the previous playerB address with the new joiner's address, effectively stealing the game slot and locking the previous player's funds in the contract.

Vulnerable Code:

function joinGameWithEth(uint256 _gameId) external payable {
Game storage game = games[_gameId];
require(game.state == GameState.Created, "Game not open to join");
require(game.playerA != msg.sender, "Cannot join your own game");
require(block.timestamp <= game.joinDeadline, "Join deadline passed");
require(msg.value == game.bet, "Bet amount must match creator's bet");
game.playerB = msg.sender;
emit PlayerJoined(_gameId, msg.sender);
// Missing: game.state = GameState.Committed; or similar state change
}

The same issue exists in joinGameWithToken function.

Impact

  1. Financial Loss: Players who are overwritten lose their bet amount, which becomes permanently locked in the contract.

  2. Denial of Service: The legitimate second player is denied participation in the game they joined.

Tools Used

  • Manual code review

  • Foundry for test case development and verification

Proof of Concept

A test case was developed that demonstrates the vulnerability:

function testMultiplePlayersCanJoin() public {
// First create a game
vm.prank(playerA);
gameId = game.createGameWithEth{value: BET_AMOUNT}(TOTAL_TURNS, TIMEOUT);
// Player B joins
vm.prank(playerB);
game.joinGameWithEth{value: BET_AMOUNT}(gameId);
// Verify playerB is set correctly
(address storedPlayerA, address storedPlayerB,,,,,,,,,,,,,, RockPaperScissors.GameState state) = game.games(gameId);
assertEq(storedPlayerB, playerB);
assertEq(uint256(state), uint256(RockPaperScissors.GameState.Created)); // Game state remains Created
// Create another player
address playerC = makeAddr("playerC");
vm.deal(playerC, 10 ether);
// PlayerC joins the same game, overwriting playerB
vm.prank(playerC);
game.joinGameWithEth{value: BET_AMOUNT}(gameId);
// Verify playerC has overwritten playerB
(,address newPlayerB,,,,,,,,,,,,,,) = game.games(gameId);
assertEq(newPlayerB, playerC);
// Verify playerB's funds are stuck in the contract
uint256 contractBalance = address(game).balance;
assertEq(contractBalance, BET_AMOUNT * 3); // PlayerA + PlayerB + PlayerC bets
}

Recommendations

There two ways to solve this issue.

  1. Update Game State: After a player joins a game update the state and add a check to prevent others joining once this state is active

  2. Check Player B Empty: Add a check to ensure playerB is empty before allowing a join:

require(game.playerB == address(0), "Game already has a second player");
Updates

Appeal created

m3dython Lead Judge about 2 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Absence of State Change on Join Allows Player B Hijacking

Game state remains Created after a player joins

Support

FAQs

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