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

Incorrect Turn Count Implementation Leads to Unplayed Final Round

0xblackadam

Summary

The Rock Paper Scissors game smart contract contains a logic error in the turn counting mechanism within the _determineWinner function. Due to an incorrect comparison operator (< instead of <=), games will always execute one fewer turn than the user-specified value in totalTurns. This results in users not receiving the full number of turns they expect, potentially affecting game outcomes and user experience.

Vulnerability Details

The vulnerability is located in the _determineWinner function at lines 526-545:

function _determineWinner(uint256 _gameId) internal {
// ... [code determining winner of current turn] ...
​
// Reset for next turn or end game
if (game.currentTurn < game.totalTurns) {
// Reset for next turn
game.currentTurn++;
game.commitA = bytes32(0);
game.commitB = bytes32(0);
game.moveA = Move.None;
game.moveB = Move.None;
game.state = GameState.Committed;
} else {
// End game
// ... [code to end game and determine overall winner] ...
}
}

The issue is in the condition game.currentTurn < game.totalTurns. When a game is created, currentTurn is initialized to 1. For a game with totalTurns set to n, the game will execute turns with currentTurn values of 1, 2, ..., n-1, and then end. The nth turn is never played because when currentTurn reaches n, the condition n < n evaluates to false.

Let's trace through an example with totalTurns set to 5:

  1. Start: currentTurn = 1

  2. After first turn completion: 1 < 5 is true, increment currentTurn to 2

  3. After second turn completion: 2 < 5 is true, increment currentTurn to 3

  4. After third turn completion: 3 < 5 is true, increment currentTurn to 4

  5. After fourth turn completion: 4 < 5 is true, increment currentTurn to 5

  6. For the fifth turn: 5 < 5 is false, so the game ends without playing the fifth turn

This means a game created with totalTurns = 5 will only play 4 actual turns.

Impact

This bug has several impacts:

  1. Reduced Game Length: Users will always get one fewer turn than they specified and expect.

  2. Potential Game Outcome Manipulation: Since the contract requires totalTurns to be odd (to avoid ties), this bug could make some games have an even number of turns, potentially resulting in ties that shouldn't occur.

  3. User Experience Degradation: Players expecting a specific number of turns will find their games shorter than anticipated.

  4. Score Calculation Impact: The final turn being skipped could affect the final score and outcome of close games.

Tools Used

  • Manual code review

  • Logic flow analysis

  • State transition simulation

Recommendations

The fix for this issue is straightforward - change the comparison operator from < to <=:

Updates

Appeal created

m3dython Lead Judge about 1 month ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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