Rock Paper Scissors

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

Any User Can Overwrite `playerB` and Steal a Game Slot — ETH and Tokens May Be Lost

Description:

The functions RockPaperScissors::joinGameWithEth() and RockPaperScissors::joinGameWithToken() allow any player to join an existing game as long as its state is Created. However, there is no validation to check whether playerB has already been assigned, which allows a new player to overwrite the existing playerB.

This introduces a critical issue: if a second player has already joined, and another user calls the join function again, they can replace the original playerB, effectively hijacking the game. If ETH or tokens were already transferred by the original playerB, these funds may become unrecoverable or locked without resolution.

Impact:

  • A malicious user can overwrite the original playerB and claim the game.

  • The overwritten playerB may lose ETH or tokens sent during their join.

  • This leads to game inconsistencies and broken trust in the protocol.

  • Could be abused to intentionally grief or sabotage other players.

Proof Of Concept:

Test: Join Overwrite Race Condition with joinGameWithEth()

  1. PlayerA creates a new game.

  2. PlayerB joins the game and transfers the required ETH.

  3. PlayerC also joins the same gameId, overwriting PlayerB.

  4. PlayerB attempts to call commitMove().

  5. The function reverts with the message "Not a player in this game".

function test_race_Condition() public {
vm.prank(playerA);
uint256 gameId = game.createGameWithEth{value: 1e18}(3, 2 days);
vm.prank(playerB);
game.joinGameWithEth{value: 1e18}(gameId);
(,address playerBJoined,,,,,,,,,,,,,, ) = game.games(gameId);
assertEq(playerBJoined, playerB);
vm.prank(playerC);
game.joinGameWithEth{value: 1e18}(gameId);
(,address playerBJoined1,,,,,,,,,,,,,, ) = game.games(gameId);
assertEq(playerBJoined1, playerC);
bytes32 saltB1 = keccak256(abi.encodePacked("01234", "56789"));
bytes32 commitPlayerB = keccak256(abi.encodePacked(uint8(RockPaperScissors.Move.Scissors), saltB1));
vm.prank(playerB);
vm.expectRevert("Not a player in this game");
game.commitMove(gameId, commitPlayerB);
}

Recommended Mitigation:

Add a validation to prevent overwriting playerB if already assigned:

function joinGameWithEth(uint256 _gameId) external payable {
Game storage game = games[_gameId];
+ require(game.playerB == address(0), "PlayerB already assigned");
...
function joinGameWithToken(uint256 _gameId) external {
Game storage game = games[_gameId];
+ require(game.playerB == address(0), "PlayerB already assigned");
...
}

Tools Used:

Manual Review Foundry

Updates

Appeal created

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