Rock Paper Scissors
First Flight #38
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Insufficient game state tracking causing potential multiple playerB entrants

cyfe45

Summary

In both the joinGameWithEth and joinGameWithToken there is insufficient updating of the game state leading to potential multiple entrants causing confusion as to who is actually playing the game.

Vulnerability Details

The RockPaperScissors game does not cater for different game states resulting in potential multiple of playerB entrants. This is due to both the joinGameWithEth and joinGameWithToken there is insufficient updating of the game state leading to potential multiple entrants causing confusion as to who is actually playing the game.

Affected code:

joinGameWithEth:

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);
}

joinGameWithToken:

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"); // @audit how does this check work?
​
// Transfer token to contract
winningToken.transferFrom(msg.sender, address(this), 1);
game.playerB = msg.sender;
emit PlayerJoined(_gameId, msg.sender);
}

Impact

Severity: High
Impact: High, as it could potentially put people off from playing the game
Likelihood: Medium, depending on popularity of the game and "malicious" players wanting to steal the game

Tools Used

Manual review of codebase.

Recommendations

Implement Atomic State Updates and Status Checks:

function joinGameWithEth(uint256 _gameId) external payable {
// ...existing validations...
require(
game.state == GameState.Created || game.state == GameState.Joined,
"Invalid state transition"
);
​
// State updates
game.playerB = msg.sender;
game.state = GameState.Joined;
game.startTime = block.timestamp;
game.currentTurn = 1; // Initialize turn counter
emit PlayerJoined(_gameId, msg.sender);
}
Updates

Appeal created

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