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

If a player has won the majority of turns, the losing player can prevent the winning player from receiving rewards

37h3rn17y2

Summary

Game does not finish even after a player has won the majority of turns. Since the opponent has essentially lost, the opponent has no incentive to continue playing. The opponent can refuse to play (commit) the remaining turns. This causes the game to be unable to reach Finished state, hence preventing the distribution of rewards to the winning player. In this game state, there are several resolution cases, all of which harms the winning player (detailed below) and results in the winning player not able to receive their rightful rewards, with 1 case even benefitting the losing player. This severely disrupts the fairness of the game.

Vulnerability Details

In RockPaperScissors::_determineWinner#L441, the game is reset for the next turn if totalTurns is not met. Even if a player has won the majority of turns, it does not end the game but resets the game for the next turn, expecting the players to continue playing the game.

However, the losing player has no incentive to continue playing and can refuse to play (commit) the remaining turns. Since the winning player cannot do anything for the game to reach Finished state before totalTurns has been played, the winning player will be unable to receive the rewards. This issue is applicable for both games created with token and ETH.

RockPaperScissors::_determineWinner#L441

// 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
address winner;
if (game.scoreA > game.scoreB) {
winner = game.playerA;
} else if (game.scoreB > game.scoreA) {
winner = game.playerB;
} else {
// This should never happen with odd turns, but just in case
// of timeouts or other unusual scenarios, handle as a tie
_handleTie(_gameId);
return;
}
​
_finishGame(_gameId, winner);
}

In this game state, there are several resolution cases, neither of which benefits the winning player.

Case 1: Player A wins majority, stalemate

  1. Player A (game creator) wins majority

  2. Player B refuses to play (commit)

  3. Player A cannot cancel game (RockPaperScissors::cancelGame) due to checks (require(game.state == GameState.Created))

  4. Both player A and player B loses out on their ETH bet

Case 2: Player B wins majority, stalemte

  1. Player B wins majority

  2. Player A (game creator) refuses to play (commit)

  3. Player B cannot cancel game (RockPaperScissors::cancelGame) due to checks (require(msg.sender == game.playerA))

  4. Both player A and player B loses out on their ETH bet
    (this is functionally similar to Case 1)

Case 3: Player wins majority, winning player use RockPaperScissors::timeoutReveal as escape hatch, benefitting the losing player

  1. Player wins majority (at turn N)

  2. Losing player refuses to play (commit) (at turn N+1)

  3. Neither player can cancel game (RockPaperScissors::cancelGame) due to the checks (require(game.state == GameState.Created)) and (require(msg.sender == game.playerA))

  4. However, during turn N when both players have committed, RockPaperScissors::cancelGame#L219 has set a revealDeadline

    // If both players have committed, set the reveal deadline
    if (game.commitA != bytes32(0) && game.commitB != bytes32(0)) {
    @> game.revealDeadline = block.timestamp + game.timeoutInterval;
    }

  5. Winning player can wait for the revealDeadline to elapse and call RockPaperScissors::timeoutReveal to rescue their bet

  6. This calls RockPaperScissors::_cancelGame which refunds the bet to both players. Winning player only receives a refund of the initial bet amount but does not get the reward they rightfully deserves and losing player does not lose the bet that they have risked (losing player gets refunded as well). This severely disrupts the fairness of the game.

PoC

Place the following into RockPaperScissorsTest.t.sol and run

forge test --mt testGameDoesNotEndAfterMajorityWins

function testGameDoesNotEndAfterMajorityWins() public {
uint256 _gameId;
uint256 playerAETHBalanceBefore;
uint256 playerAETHBalanceAfter;
uint256 playerBETHBalanceBefore;
uint256 playerBETHBalanceAfter;
​
_gameId = createAndJoinGame();
playerAETHBalanceBefore = playerA.balance;
playerBETHBalanceBefore = playerB.balance;
​
// Player A has already won majority of turns
playTurn(_gameId, RockPaperScissors.Move.Rock, RockPaperScissors.Move.Scissors);
playTurn(_gameId, RockPaperScissors.Move.Rock, RockPaperScissors.Move.Scissors);
​
// Player B refuses to play (commit) the remaining turns
​
// Player A has no way to cancel the game
vm.prank(playerA);
vm.expectRevert();
game.cancelGame(_gameId);
​
// Player A does not their receive rewards
playerAETHBalanceAfter = playerA.balance;
assertEq(playerAETHBalanceBefore, playerAETHBalanceAfter);
// Game state is stuck in "Committed"
(,,uint256 betAmount,,,,,,,,,,,,,RockPaperScissors.GameState state) = game.games(_gameId);
assertEq(uint8(state), uint8(RockPaperScissors.GameState.Committed));
​
// Case 3 Escape Hatch
// Player A waits for the revealDeadline (set in 2nd turn) to elapse
vm.warp(block.timestamp + TIMEOUT + 1);
// Player A calls timeoutReveal as escape hatch to recover some funds
vm.prank(playerA);
game.timeoutReveal(_gameId);
​
// Player A does not their receive rewards (only refunded bet)
playerAETHBalanceAfter = playerA.balance;
uint256 PROTOCOL_FEE_PERCENT = 10;
uint256 playerARightfulWinnings = (betAmount * 2 * (100 - PROTOCOL_FEE_PERCENT)) / 100;
assertLt(playerAETHBalanceAfter - playerAETHBalanceBefore, playerARightfulWinnings);
assertEq(playerAETHBalanceAfter - playerAETHBalanceBefore, betAmount);
​
// Player B got refunded their bet eventhough they lost
playerBETHBalanceAfter = playerB.balance;
assertEq(playerBETHBalanceAfter - playerBETHBalanceBefore, betAmount);
​
}

Impact

Impact: High. Game is stuck and winning player cannot receive rewards
Likelihood: High. It is common occurrence in games to have majority wins before totalTurns has been played. If a player has lost before totalTurns, there is no incentive to continue playing. Additionally, the player can choose to use this vulnerability to exert revenge as griefing attack to prevent winning player from receiving rewards
Severity: High

Tools Used

Manual review

Recommendations

End the game when a player has won a majority of turns.

function _determineWinner(uint256 _gameId) internal {
Game storage game = games[_gameId];
​
address turnWinner = address(0);
​
// Rock = 1, Paper = 2, Scissors = 3
if (game.moveA == game.moveB) {
// Tie, no points
turnWinner = address(0);
} else if (
(game.moveA == Move.Rock && game.moveB == Move.Scissors)
|| (game.moveA == Move.Paper && game.moveB == Move.Rock)
|| (game.moveA == Move.Scissors && game.moveB == Move.Paper)
) {
// Player A wins
game.scoreA++;
turnWinner = game.playerA;
} else {
// Player B wins
game.scoreB++;
turnWinner = game.playerB;
}
​
emit TurnCompleted(_gameId, turnWinner, game.currentTurn);
​
// Reset for next turn or end game
if (game.currentTurn < game.totalTurns) {
+ // Check if a player has won majority of the turns
+ uint256 public constant PRECISION = 100;
+ uint256 public constant MAJORITY = 50; // 50%
+ bool playerAalreadyWonMajority = game.scoreA * PRECISION / game.totalTurns > MAJORITY
+ bool playerBalreadyWonMajority = game.scoreB * PRECISION / game.totalTurns > MAJORITY
+
+ // If playerA has already won the majority of turns, finish the game with player A as the winner
+ // If playerB has already won the majority of turns, finish the game with player B as the winner
+ // If neither player has won majority, reset for next turn
+ if (playerAalreadyWonMajority) {
+ _finishGame(_gameId, game.playerA)
+ } else if (playerBalreadyWonMajority) {
+ _finishGame(_gameId, game.playerB)
+ } else {
// 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
address winner;
if (game.scoreA > game.scoreB) {
winner = game.playerA;
} else if (game.scoreB > game.scoreA) {
winner = game.playerB;
} else {
_handleTie(_gameId);
return;
}
​
_finishGame(_gameId, winner);
}
}
Updates

Appeal created

m3dython Lead Judge about 2 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Player B cannot cancel a game if Player A becomes unresponsive after Player B joins

Protocol does not provide a way for Player B to exit a game and reclaim their stake if Player A stops participating

37h3rn17y2 Submitter
about 2 months ago
37h3rn17y2 Submitter
about 2 months ago
m3dython Lead Judge
about 2 months ago
m3dython Lead Judge about 2 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Player B cannot cancel a game if Player A becomes unresponsive after Player B joins

Protocol does not provide a way for Player B to exit a game and reclaim their stake if Player A stops participating

Support

FAQs

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