Rock Paper Scissors
First Flight #38
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Multiple Player Bs

feranmiola

Summary

A critical vulnerability exists in the Rock-Paper-Scissors game smart contract that allows multiple players to join the same game as "Player B," resulting in lost funds. The contract fails to verify if a game already has a Player B before allowing new players to join, allowing an unlimited number of users to join the same game, with only the last one being recognized as the official Player B while all previous players lose their staked ETH or tokens.

Vulnerability Details

The vulnerability exists in both the joinGameWithEth and joinGameWithToken functions, which lack a crucial check to verify if the game already has a Player B:

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; // Overwrites any existing Player B
emit PlayerJoined(_gameId, msg.sender);
}

The same issue exists in the joinGameWithToken function. When multiple players attempt to join the same game, the contract:

  1. Accepts ETH or tokens from each player

  2. Overwrites the game.playerB address each time

  3. Does not refund previous players

This issue is made worse by the reward calculation in the finishGame function:

//.....
uint256 totalPot = game.bet * 2;
//.....

This calculation assumes exactly two players have contributed funds. However, with this vulnerability, the contract could collect funds from dozens of players while only accounting for two players' worth of stakes in the reward distribution.

Impact

  • Players who attempt to join a game that already has a Player B will lose their funds permanently.

  • The contract will accumulate more funds than it can distribute, creating an imbalance.

  • The game is designed for exactly two players, but this vulnerability allows more players to contribute funds.

  • Players losing funds will lose trust in the protocol.

Tools Used

Manual code review

Recommendations

Add a critical check in both join functions to verify that no Player B has already joined:

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(game.playerB == address(0), "Game already has a second player"); // Add this check
require(msg.value == game.bet, "Bet amount must match creator's bet");
​
game.playerB = msg.sender;
emit PlayerJoined(_gameId, msg.sender);
}

The same check should be added to the joinGameWithToken function. This ensures that once a game has a Player B, no additional players can join and lose their funds.

Alternatively, you could consider changing the game state after a Player B joins:

game.playerB = msg.sender;
game.state = GameState.Active; // Add a new state or use an existing appropriate state
emit PlayerJoined(_gameId, msg.sender);
Updates

Appeal created

m3dython Lead Judge 4 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.