Rock Paper Scissors

First Flight #38
Beginner FriendlySolidity
100 EXP
View results
Submission Details
Severity: medium
Invalid

(Medium) Commit-Reveal Timeout Allows Forced Cancellation

Title

(Medium) Commit-Reveal Timeout Allows Forced Cancellation - A player can avoid losing their bet/stake by failing to reveal their move, forcing a game cancellation and refund if the opponent also doesn't reveal. Breaks fairness and wastes opponent's time/gas.

Summary

The Rock Paper Scissors game uses a commit-reveal scheme to prevent players from cheating their moves. However, the implementation of the timeout mechanism for the reveal phase allows a player to commit a move and then, if they anticipate losing or simply wish to grief the opponent, deliberately fail to reveal their move. If the opponent also fails to reveal their move before the deadline (which is likely if they are waiting for the first player to reveal, or are also trying to game the system), the game is cancelled, and both players' bets/stakes are refunded. This breaks the fairness of the game, allowing a player to avoid losing at no significant cost.

Vulnerability Details

The commit-reveal pattern involves two phases:

  1. Commit: Players submit a hash of their move and a secret salt (keccak256(move + salt)). This proves they decided on a move before seeing the opponent's move, but doesn't reveal the move itself.

  2. Reveal: Players submit their actual move and salt. The contract verifies the hash matches the commitment. Once both reveal, the winner is determined.

A time limit (timeoutInterval) is set for the reveal phase. If a player fails to reveal within this time, the timeoutReveal function (callable by anyone) can be used to finalize the game based on the reveal status.

The vulnerability lies in the timeoutReveal function's handling of the scenario where neither player reveals their move before the deadline.

https://github.com/CodeHawks-Contests/2025-04-rock-paper-scissors/blob/main/src/RockPaperScissors.sol#L262-L285

/**
* @notice Claim win if opponent didn't reveal in time
* @param _gameId ID of the game
*/
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"); // BUG: Anyone can call, but this check restricts it to players.
require(game.state == GameState.Committed, "Game not in reveal phase");
require(block.timestamp > game.revealDeadline, "Reveal phase not timed out yet");
// If player calling timeout has revealed but opponent hasn't, they win
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"); // Both revealed, or caller didn't reveal but opponent did
}
}
  1. The line else if (!playerARevealed && !playerBRevealed) { _cancelGame(_gameId); } (L283-285) is the problematic logic. If both players fail to reveal, the game is cancelled, and _cancelGame (L547-574) refunds the full bet/token stake to both players.

  2. This creates a scenario where a player who is likely to lose (or simply doesn't want to continue) can simply refrain from revealing. If the opponent also waits to reveal, or also fails to reveal, the game cancels, and the malicious player gets their stake back, effectively avoiding a loss at the cost of the opponent's time and gas used in the commit phase.

  3. Self-Correction during analysis: The test suite (RockPaperScissorsTest.t.sol) contains a test testTimeoutRevealNoReveals, which explicitly tests this scenario, confirms the game state becomes Cancelled, and asserts that both players receive refunds. This confirms the observed behavior from the code analysis. The test implicitly demonstrates the potential for griefing/avoiding loss.

Exploitation Procedure

This exploitation procedure demonstrates how a player can force a cancellation to avoid losing if the opponent also doesn't reveal.

  1. Two players, Alice (Player A) and Bob (Player B), create and join an ETH or Token game. Both players commit their moves (commitMove).

  2. Neither Alice nor Bob reveals their move immediately. (This is common behavior as both might wait for the other).

  3. The revealDeadline passes without either player calling revealMove.

  4. Alice (or Bob, or ideally anyone if L269 was removed, but the current code restricts it to players) calls the timeoutReveal function for the game.

  5. The contract checks !playerARevealed && !playerBRevealed (as neither called revealMove) and finds it true.

  6. The contract calls _cancelGame, setting the game state to Cancelled.

The contract refunds the ETH bets or returns the token stakes to both Alice and Bob.
Alice, by simply not revealing, has avoided the possibility of losing the game (and her bet/stake) if her committed move was weak or if Bob's unrevealed move was strong. She gets her stake back, forcing a draw-by-cancellation instead of a potential loss.

Impact

This vulnerability undermines the fairness of the game. A player can:

  1. Commit a move.

  2. Wait to see if the opponent reveals first (or if the reveal deadline is approaching).

  3. If they anticipate losing (perhaps due to off-chain information, or simply observing network traffic if the commit/reveal is not perfectly shielded), or simply want to grief the opponent, they can choose not to reveal their move before the deadline.

  4. If the opponent also fails to reveal within the deadline, either player (restricted to players by L269, but still exploitable by either player) can call timeoutReveal, which triggers _cancelGame.

  5. Both players receive a full refund of their ETH bet or token stake.

The malicious player avoids losing their stake and prevents the honest player from winning, forcing a cancellation instead of a legitimate game outcome. This wastes the honest player's time and transaction costs for the commit phase.

Tools Used

  1. Manual code review

  2. AI for analysis and summarization

Recommendations

To fix this vulnerability and ensure game fairness, players who fail to reveal within the deadline should be penalized, not refunded. The common approach is to treat failure to reveal as a loss of that turn's stake. If neither player reveals, both should lose their stake (which could be sent to the contract's fee balance or burned).

Updates

Appeal created

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

Support

FAQs

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