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

Inconsistent logic in the _determineWinner() function allows player to manipulate game outcome.

legax

Summary

The game state in the _determineWinner() function is incorrectly set to GameState.Committed after each round, rather than reverting to GameState.Created. This can lead to unintended game flow behavior.

Vulnerability Details

In the provided code snippet, the game state transitions directly to GameState.Committed after each round, assuming the current turn is less than the total number of turns:

if (game.currentTurn < game.totalTurns) {
// Reset for the next turn
game.currentTurn++;
game.commitA = bytes32(0);
game.commitB = bytes32(0);
game.moveA = Move.None;
game.moveB = Move.None;
game.state = GameState.Committed; // Incorrect state transition
}

However, if a player chooses not to commit their move, the revealDeadline is not reinitialized:

function commitMove(uint256 _gameId, bytes32 _commitHash) external {
--- SNIPPED ---
// If both players have committed, set the reveal deadline
if (game.commitA != bytes32(0) && game.commitB != bytes32(0)) {
game.revealDeadline = block.timestamp + game.timeoutInterval;
}
--- SNIPPED ---

As a result, a player can successfully call the timeoutReveal function to end the game prematurely.

function timeoutReveal(uint256 _gameId) external {
Game storage game = games[_gameId];
// Every check will pass
require(msg.sender == game.playerA || msg.sender == game.playerB, "Not a player in this game");
require(game.state == GameState.Committed, "Game not in reveal phase");
require(block.timestamp > game.revealDeadline, "Reveal phase not timed out yet");
​
// Determine winner based on reveals
bool playerARevealed = game.moveA != Move.None;
bool playerBRevealed = game.moveB != Move.None;
​
if (msg.sender == game.playerA && playerARevealed && !playerBRevealed) {
// Player A wins by timeout
_finishGame(_gameId, game.playerA);
} else if (msg.sender == game.playerB && playerBRevealed && !playerARevealed) {
// Player B wins by timeout
_finishGame(_gameId, game.playerB);
} else if (!playerARevealed && !playerBRevealed) {
// Neither player revealed, cancel the game and refund
_cancelGame(_gameId);
} else {
revert("Invalid timeout claim");
}
}

Impact

This vulnerability allows a player to:

  • Force a win by committing a move then calling timeoutReveal and winning the game due to the opponent failing to reveal their move (which should not happen before committing).

  • Cancel the game unfairly by triggering a timeout, even if they haven’t completed the required game actions.

Recommendations

After each turn, reset the game.state back to GameState.Created:

if (game.currentTurn < game.totalTurns) {
// Reset for the next turn
game.currentTurn++;
game.commitA = bytes32(0);
game.commitB = bytes32(0);
game.moveA = Move.None;
game.moveB = Move.None;
game.state = GameState.Created; // Correct state transition
}
Updates

Appeal created

m3dython Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational

Code suggestions or observations that do not pose a direct security risk.

m3dython Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational

Code suggestions or observations that do not pose a direct security risk.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.