Rock Paper Scissors

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

(low) Commitment Hash Replay -Allows reusing `(move, salt)` across turns/games, potentially leaking information.

Title

(low) Commitment Hash Replay -Allows reusing (move, salt) across turns/games, potentially leaking information.

Summary

The commitment hash generated by keccak256(abi.encodePacked(move, _salt)) in revealMove and checked against the hash provided in commitMove does not include unique identifiers for the specific game or turn. This allows a player to reuse the same (move, salt) pair, and therefore the same commitment hash, across different turns within the same game or across different games. While this doesn't allow direct cheating, it weakens the commitment scheme by potentially leaking information about a player's strategy if they reuse commitments and allows for trivial commitments (e.g., always using the same salt for 'Rock').

Vulnerability Details

Location: src/RockPaperScissors.sol, Function: revealMove, Line: 238
https://github.com/CodeHawks-Contests/2025-04-rock-paper-scissors/blob/main/src/RockPaperScissors.sol#L229-L256

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");
require(block.timestamp <= game.revealDeadline, "Reveal phase timed out");
require(_move >= 1 && _move <= 3, "Invalid move");
Move move = Move(_move);
// VULNERABILITY: Hash does not include gameId or currentTurn
bytes32 commit = keccak256(abi.encodePacked(move, _salt)); // Highlighted line
if (msg.sender == game.playerA) {
require(commit == game.commitA, "Hash doesn't match commitment");
require(game.moveA == Move.None, "Move already revealed");
game.moveA = move;
} else {
require(commit == game.commitB, "Hash doesn't match commitment");
require(game.moveB == Move.None, "Move already revealed");
game.moveB = move;
}
emit MoveRevealed(_gameId, msg.sender, move, game.currentTurn);
// If both players have revealed, determine the winner for this turn
if (game.moveA != Move.None && game.moveB != Move.None) {
_determineWinner(_gameId);
}
}
  1. When you commit to a move (like Rock), you create a secret code (hash) using your move and a random value (salt). Later, you reveal the move and the salt, and the contract checks if they match the secret code you committed earlier. The issue is that the secret code only depends on the move and the salt, not on which specific game or which turn it is. This means you could use the exact same move and salt (and thus the same secret code) in turn 1 of game 1, turn 2 of game 1, or turn 1 of game 5. If an opponent observes you using the same secret code repeatedly, they might guess you're making the same move, even without knowing the salt.

  2. The keccak256 hash used for the commit-reveal scheme incorporates only the move and _salt inputs (abi.encodePacked(move, _salt)). It lacks domain separation parameters like _gameId and game.currentTurn. Without these unique identifiers per commitment context, the same (move, _salt) pair will produce an identical hash regardless of the game or turn number, allowing the hash to be validly submitted and revealed in multiple distinct commitment slots.

Impact

  1. Information Leakage: An observant opponent playing multiple games or turns against the same player might notice the reuse of commitment hashes. While they can't know the move without the salt, seeing the same hash could signal that the player is reusing a strategy or salt, potentially influencing the opponent's own strategy.

  2. Reduced Perceived Randomness/Security: Allows players to trivially generate commitments (e.g., always use salt = 0x01 for Rock, salt = 0x02 for Paper, etc.) across all games, which undermines the purpose of using unique salts for each commitment.

  3. Business Impact: Low. Does not directly enable theft or cheating but slightly weakens the game's cryptographic guarantees and could make sophisticated players feel the commitment scheme is less robust.

Exploitation Procedure

Exploitation Procedure
Prerequisites: Two active games (Game ID 0 and Game ID 1) involving Player A and Player B, both in the commit phase for Turn 1.

  1. Player A Action (Game 0, Turn 1):

  2. Choose Move: Rock (uint8 value 1)

  3. Choose Salt: bytes32 constant REUSED_SALT = keccak256("my-reused-salt");

  4. Calculate Commit: bytes32 commitHash = keccak256(abi.encodePacked(uint8(1), REUSED_SALT));

  5. Call game.commitMove(0, commitHash) as Player A.

  6. Player A Action (Game 1, Turn 1):

  7. Use the same move and salt.

  8. Calculate the same commit hash: bytes32 commitHash = keccak256(abi.encodePacked(uint8(1), REUSED_SALT));

  9. Call game.commitMove(1, commitHash) as Player A.

Observation: Player B (or anyone observing the blockchain) can see that Player A submitted the identical commitHash for both Game 0 and Game 1.

  1. Player A Action (Game 0, Turn 1 Reveal):

  2. Call game.revealMove(0, 1, REUSED_SALT) as Player A. This succeeds.

  3. Player A Action (Game 1, Turn 1 Reveal):

  4. Call game.revealMove(1, 1, REUSED_SALT) as Player A. This also succeeds using the same move and salt.

Tools Used

  1. Manual Review

  2. AI for report generation and impact analysis.

Recommendations

  1. Include _gameId and the currentTurn within the commitment hash to ensure uniqueness for each commitment slot. Modify the hash calculation in revealMove and ensure players use this new format when generating their commitments for commitMove.

Updates

Appeal created

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

Lack of Salt Uniqueness Enforcement

The contract does not enforce salt uniqueness

Support

FAQs

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