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

TimeoutReveal can be exploited before `revealDeadline` is set

0xbyteknight

Summary

Players can prematurely cancel a game using the timeoutReveal function even when the game hasn't entered the reveal phase. This is possible because the revealDeadline may not be initialized yet if only one player has committed their move.

Vulnerability Details

The timeoutReveal function is designed to resolve games when one or both players fail to reveal their moves after committing. However, the function lacks a guard to ensure that the reveal phase has actually started.
The following check in the function is vulnerable:

require(block.timestamp > game.revealDeadline, "Reveal phase not timed out yet");

Since revealDeadline is initialized only after both players commit, its default value is 0. This allows any player to call timeoutReveal() after the game has started but before both commitments are submitted, causing an unintended cancellation.

Impact

  • Premature game cancellation

  • Disruption of game flow

  • Potential abuse to escape from a losing position

Tools Used

  • Manual code review

  • Foundry

PoC

The following test simulates a scenario where Player B commits their move but Player A calls timeoutReveal() before the reveal phase has officially started. Since revealDeadline is still zero, the timeout succeeds and the game is improperly cancelled.

The following test_playerAUsesTimeoutRevealBeforeDeadlineSet test function can be placed in the RockPaperScissorsTest.t.sol file:

function test_playerAUsesTimeoutRevealBeforeDeadlineSet() public {
// Player A creates a new ETH-based game
vm.prank(playerA);
gameId = game.createGameWithEth{value: BET_AMOUNT}(1, TIMEOUT);
​
// Player B joins the game
vm.prank(playerB);
game.joinGameWithEth{value: BET_AMOUNT}(gameId);
​
// Time moves forward slightly
vm.warp(block.timestamp + 1 minutes);
​
// Player B commits a move
bytes32 saltB = keccak256(abi.encodePacked("salt for player B"));
bytes32 commitB = keccak256(abi.encodePacked(uint8(RockPaperScissors.Move.Rock), saltB));
​
vm.prank(playerB);
emit MoveCommitted(gameId, playerB, 1);
game.commitMove(gameId, commitB);
​
// Player A triggers timeoutReveal before the revealDeadline has been initialized
vm.prank(playerA);
game.timeoutReveal(gameId);
​
// Check that game was improperly cancelled
(, , , , , , , , uint256 currentTurn, , , RockPaperScissors.Move moveA, RockPaperScissors.Move moveB, uint8 scoreA, uint8 scoreB, RockPaperScissors.GameState state) = game.games(gameId);
​
assertEq(uint256(moveA), uint256(RockPaperScissors.Move.None));
assertEq(uint256(moveB), uint256(RockPaperScissors.Move.None));
assertEq(scoreA, 0);
assertEq(scoreB, 0);
assertEq(uint256(state), uint256(RockPaperScissors.GameState.Cancelled));
}

Recommendations

Add a guard to ensure the reveal deadline has been set before allowing timeout:

function timeoutReveal(uint256 _gameId) 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");
+ require(game.revealDeadline != 0, "Reveal phase has not started");
require(block.timestamp > game.revealDeadline, "Reveal phase not timed out yet");
...
}

This ensures that the timeout logic can only execute after both players have committed and the reveal phase is officially active.

Updates

Appeal created

m3dython Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Design choice
m3dython Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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