Submission Details
Severity:
Commit-Reveal Griefing Attack
Summary
The RockPaperScissors smart contract implements a commit-reveal scheme to ensure fairness in a two-player game. However, the current implementation is vulnerable to commit-reveal griefing, where one player can stall the game after seeing the opponent's revealed move. This results in a denial of service for the other player and halts game progression.
Vulnerability Details
The vulnerability lies in the fact that one player can choose not to reveal their committed move after the opponent has revealed theirs, effectively stalling the game. This griefing strategy is possible because there are no incentives or penalties around the reveal phase.
Exploit Scenario:
-
Player A commits Rock
-
Player B commits Paper (which beats Rock)
-
Player B reveals first - revealing Paper
-
Player A refuses to reveal, stalling the game
Proof of Concept
The following Foundry test demonstrates how to stalling the game:
// test/RockPaperScissorsVuln2.t.sol
import "forge-std/Test.sol";
import "../src/RockPaperScissors.sol";
contract CommitRevealTimingAbuse is Test {
RockPaperScissors public game;
// Events for testing
event GameCreated(
uint256 indexed gameId,
address indexed creator,
uint256 bet,
uint256 totalTurns
);
event PlayerJoined(uint256 indexed gameId, address indexed player);
event MoveCommitted(
uint256 indexed gameId,
address indexed player,
uint256 currentTurn
);
event MoveRevealed(
uint256 indexed gameId,
address indexed player,
RockPaperScissors.Move move,
uint256 currentTurn
);
event TurnCompleted(
uint256 indexed gameId,
address winner,
uint256 currentTurn
);
event GameFinished(uint256 indexed gameId, address winner, uint256 prize);
event GameCancelled(uint256 indexed gameId);
event JoinTimeoutUpdated(uint256 oldTimeout, uint256 newTimeout);
event FeeCollected(uint256 gameId, uint256 feeAmount);
event FeeWithdrawn(address indexed admin, uint256 amount);
// Contracts
// RockPaperScissors public game;
WinningToken public token;
// Test accounts
address public admin;
address public playerA;
address public playerB;
// Test constants
uint256 constant BET_AMOUNT = 0.1 ether;
uint256 constant TIMEOUT = 10 minutes;
uint256 constant TOTAL_TURNS = 3; // Must be odd
// Game ID for tests
uint256 public gameId;
// Setup before each test
function setUp() public {
// Set up addresses
admin = address(this);
playerA = makeAddr("playerA");
playerB = makeAddr("playerB");
// Fund the players
vm.deal(playerA, 10 ether);
vm.deal(playerB, 10 ether);
// Deploy contracts
game = new RockPaperScissors();
token = WinningToken(game.winningToken());
// Mint some tokens for players for token tests
vm.prank(address(game));
token.mint(playerA, 10);
vm.prank(address(game));
token.mint(playerB, 10);
}
// Helper function to create and join a game
function createAndJoinGame() internal returns (uint256) {
vm.prank(playerA);
uint256 id = game.createGameWithEth{value: BET_AMOUNT}(
TOTAL_TURNS,
TIMEOUT
);
vm.prank(playerB);
game.joinGameWithEth{value: BET_AMOUNT}(id);
return id;
}
// Test commit-reveal timing abuse
function testCommitRevealAbuse() public {
gameId = createAndJoinGame();
// Player A commits Rock
bytes32 saltA = keccak256(abi.encodePacked("salt for player A"));
bytes32 commitA = keccak256(
abi.encodePacked(uint8(RockPaperScissors.Move.Rock), saltA)
);
vm.prank(playerA);
game.commitMove(gameId, commitA);
// Player B commits Paper (which beats Rock)
bytes32 saltB = keccak256(abi.encodePacked("salt for player B"));
bytes32 commitB = keccak256(
abi.encodePacked(uint8(RockPaperScissors.Move.Paper), saltB)
);
vm.prank(playerB);
game.commitMove(gameId, commitB);
// Player B reveals first — revealing Paper
vm.prank(playerB);
game.revealMove(gameId, uint8(RockPaperScissors.Move.Paper), saltB);
// Player A refuses to reveal, stalling the game
(
,
,
,
,
,
,
,
,
uint256 currentTurn,
,
,
RockPaperScissors.Move moveA,
RockPaperScissors.Move moveB,
uint8 scoreA,
uint8 scoreB,
RockPaperScissors.GameState state
) = game.games(gameId);
// Paper move is already revealed
assertEq(uint256(moveB), uint8(RockPaperScissors.Move.Paper));
// Player A has not revealed — move is still None
assertEq(uint256(moveA), uint8(RockPaperScissors.Move.None));
// Game is stuck at current turn
assertEq(currentTurn, 1);
// Score hasn’t changed
assertEq(scoreA, 0);
assertEq(scoreB, 0);
// State remains stuck
assertEq(
uint256(state),
uint256(RockPaperScissors.GameState.Committed)
);
}
}
Impact
Tools Used
Recommendations
Add commitDeadline:
// Game structure
struct Game {
address playerA; // Creator of the game
address playerB; // Second player to join
uint256 bet; // Amount of ETH bet
uint256 timeoutInterval; // Time allowed for reveal phase
uint256 revealDeadline; // Deadline for revealing moves
+ uint256 commitDeadline; // Deadline for committing moves
uint256 creationTime; // When the game was created
uint256 joinDeadline; // Deadline for someone to join the game
uint256 totalTurns; // Total number of turns in the game
uint256 currentTurn; // Current turn number
bytes32 commitA; // Hashed move from player A
bytes32 commitB; // Hashed move from player B
Move moveA; // Revealed move from player A
Move moveB; // Revealed move from player B
uint8 scoreA; // Score for player A
uint8 scoreB; // Score for player B
GameState state; // Current state of the game
}
To fix the vulnerability, modify the revealMove function to strictly enforce time limits and implement mechanisms like automatic forfeit or score assignment if one player fails to reveal in time. Below is the secured version of revealMove:
@@
/**
* @notice Reveal committed move
* @param _gameId ID of the game
* @param _move Player's move (1=Rock, 2=Paper, 3=Scissors)
* @param _salt Random salt used in the commit phase
*/
- function revealMove(uint256 _gameId, uint8 _move, bytes32 _salt) external {
+ // Reveal a move (hashing the move and salt)
+ 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");
+ // Ensure that the caller is one of the players in the game
+ require(
+ msg.sender == game.playerA || msg.sender == game.playerB,
+ "Not a player in this game"
+ );
+
+ // Ensure the game is in the 'Committed' state
+ require(game.state == GameState.Committed, "Game not in reveal phase");
+
+ // Ensure the reveal phase has not timed out
+ require(
+ block.timestamp <= game.revealDeadline,
+ "Reveal phase timed out"
+ );
+
+ // Ensure the move is valid
+ require(_move >= 1 && _move <= 3, "Invalid move");
Move move = Move(_move);
bytes32 commit = keccak256(abi.encodePacked(move, _salt));
- 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;
- }
+ // Check if the player is Player A
+ if (msg.sender == game.playerA) {
+ require(
+ commit == game.commitA,
+ "Hash doesn't match commitment for player A"
+ );
+ require(
+ game.moveA == Move.None,
+ "Move already revealed for player A"
+ );
+ game.moveA = move;
+ } else {
+ // Check if the player is Player B
+ require(
+ commit == game.commitB,
+ "Hash doesn't match commitment for player B"
+ );
+ require(
+ game.moveB == Move.None,
+ "Move already revealed for player B"
+ );
+ game.moveB = move;
+ }
- emit MoveRevealed(_gameId, msg.sender, move, game.currentTurn);
+ // Emit event for move revelation
+ emit MoveRevealed(_gameId, msg.sender, move, game.currentTurn);
- // If both players have revealed, determine the winner for this turn
+ // If both players have revealed their moves, determine the winner
if (game.moveA != Move.None && game.moveB != Move.None) {
_determineWinner(_gameId);
}
}
Updates
Appeal created
m3dython 4 months ago
Submission Judgement Published Reason: Non-acceptable severity
Assigned finding tags:
Informational
Code suggestions or observations that do not pose a direct security risk.
m3dython 4 months ago
Submission Judgement Published Reason: Non-acceptable severity
Assigned finding tags:
Informational
Code suggestions or observations that do not pose a direct security risk.
Rewards breakdown
nSLOC
349This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.