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

Player can force win by blocking opponent's turn exploiting lack of reveal deadline reset

vladich0x

Summary

Player can force win by blocking opponent's turn exploiting lack of reveal deadline reset

Vulnerability Details

The function RockPaperScissors::_determineWinner() implements the complex logic covering turn winner determination, reset after each turn and game completion along with distribution of rewards/refunding. After turn winner determination in the top of the implementaiton, following is the game settings reset, preparing for the next turn. Current turn is incremented, player commits are put in initial state, along with the revealed moves. At the end game state is moved back to GameState.Committed. Missing is only revealDeadline reset, which could cause the entire game to be blocked further and one of the players to force win. If the players complete quickly the current turn before revealDeadline expiration, when the next turn starts, each player is able to block the other one from commiting its hashed value. The fragile validation of function RockPaperScissors::revealMove() could be exploited from anyone of the players by sequentially committing the hashed value and further revealing the move. All the checks will be passed in case the revealDeadline still not expired. When the opponent tries later to commit hashed move, the function RockPaperScissors::commitMove() will revert with Moves already committed for this turn blocking the player.

Function RockPaperScissors::_determineWinner():

// 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;
// Not resetting revealDeadline allows to block entire game
} else {

Function RockPaperScissors::revealMove()

function revealMove(uint256 _gameId, uint8 _move, bytes32 _salt) external {
Game storage game = games[_gameId];
​
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");
// the next line unlocks revealing move after game stats reset at the end of the turn
require(block.timestamp <= game.revealDeadline, "Reveal phase timed out");
require(_move >= 1 && _move <= 3, "Invalid move");

Function RockPaperScissors::commitMove()

if (game.currentTurn == 1 && game.commitA == bytes32(0) && game.commitB == bytes32(0)) {
// First turn, first commits
require(game.playerB != address(0), "Waiting for player B to join");
game.state = GameState.Committed;
} else {
// Later turns or second player committing
require(game.state == GameState.Committed, "Not in commit phase");
require(game.moveA == Move.None && game.moveB == Move.None, "Moves already committed for this turn"); // the opponent could not make commit anymore since the other player has already revealed his move
}

Impact

Мissing reset of revealDeadline along with other game stats at the end of the turn:

  • unlocks RockPaperScissors::revealMove for a window until revealDeadline is expired

  • each of the players can do a commit, followed sequentially by reveal (no need to wait for opponent to commit in order revealTimeout is set and revealing be enabled)

  • lets assume playerA has done a sequential commit + reveal

  • further attempts of opponent (playerB) to make a commit will fail

  • playerA could wait util revealDeadline expired and call funciton RockPaperScissors::timeoutReveal() in order to force the win and collect the entire reward

Tools Used

Manual review
Foundry

PoC

Add the following test to RockPaperScissors.t.sol.

function testBlockOpponentExpoitingRevealDeadline() public {
gameId = createAndJoinGame();
​
// turn 1: A=Rock, B=Paper (B wins)
playTurn(gameId, RockPaperScissors.Move.Rock, RockPaperScissors.Move.Paper);
​
// turn 2:
bytes32 saltA = keccak256(abi.encodePacked("salt for player A"));
bytes32 commitA = keccak256(abi.encodePacked(uint8(RockPaperScissors.Move.Rock), saltA));
// player A commits move
vm.prank(playerA);
game.commitMove(gameId, commitA);
​
// player A reveals move
vm.prank(playerA);
game.revealMove(gameId, uint8(RockPaperScissors.Move.Rock), saltA);
​
bytes32 saltB = keccak256(abi.encodePacked("salt for player B"));
bytes32 commitB = keccak256(abi.encodePacked(uint8(RockPaperScissors.Move.Paper), saltB));
​
// player B attempts to commit move but reverts
vm.prank(playerB);
vm.expectRevert("Moves already committed for this turn");
game.commitMove(gameId, commitB);
}

Recommendations

Reset revealDeadline at the end of the turn in function RockPaperScissors::_determineWinner()

// 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;
+ revealDeadline = 0;
} else {
Updates

Appeal created

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