Rock Paper Scissors

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

Griefing via `cancelGame` Due to Insufficient State Check

Summary

The RockPaperScissors.sol contract provides a cancelGame function intended for the game creator (Player A) to cancel a game and retrieve their stake. However, this function only validates that the game.state is GameState.Created and that the caller is game.playerA. Crucially, the functions used by the second player to join (joinGameWithEth, joinGameWithToken) set game.playerB but do not transition the game.state away from Created. This discrepancy allows Player A to successfully call cancelGame even after Player B has already joined the game and committed their stake (ETH or Token). While the underlying _cancelGame function correctly refunds both players in this scenario, the ability for Player A to unilaterally cancel a game after Player B has joined constitutes a griefing vector, enabling unfair game termination.

Vulnerability Details

  1. cancelGame Function Logic: This function allows cancellation based on two checks:

    • require(game.state == GameState.Created, "Game must be in created state");

    • require(msg.sender == game.playerA, "Only creator can cancel");

  2. joinGame... Function Behavior: The joinGameWithEth and joinGameWithToken functions successfully register Player B by setting game.playerB = msg.sender and taking their stake. However, they do not modify game.state, leaving it as GameState.Created.

  3. Missing Check in cancelGame: The cancelGame function lacks a check to verify if Player B has already joined (e.g., require(game.playerB == address(0), "Cannot cancel after player B joined");).

  4. Exploitation: Because the game.state remains Created even after Player B joins, Player A can still satisfy the conditions of the cancelGame function and successfully execute it.

  5. Outcome: The _cancelGame internal function is called. It sets the state to Cancelled and correctly refunds both Player A and Player B (since game.playerB != address(0)).

Proof Of Concept

The provided test case below demonstrates this vulnerability

// ==================== GRIEFING VIA CANCELGAME TEST ====================
/**
* @notice Tests that Player A can cancel the game even after Player B has joined.
* This is possible because joinGame doesn't change state from Created,
* and cancelGame only checks for Created state, not if playerB exists.
*/
function testGriefingViaCancelGameAfterJoin() public {
// --- Setup: Create and Join an ETH Game ---
gameId = createAndJoinGame(); // Helper creates and has B join
// --- Assertions After Join (Before Cancel) ---
(
address storedPlayerA_before,
address storedPlayerB_before,
, // bet
, // timeoutInterval
, // revealDeadline
, // creationTime
, // joinDeadline
, // totalTurns
, // currentTurn
, // commitA
, // commitB
, // moveA
, // moveB
, // scoreA
, // scoreB
RockPaperScissors.GameState state_before
) = game.games(gameId);
assertEq(storedPlayerA_before, playerA, "Player A should be set");
assertEq(storedPlayerB_before, playerB, "Player B should be set after joining");
assertEq(uint256(state_before), uint256(RockPaperScissors.GameState.Created), "State should still be Created after join");
assertEq(address(game).balance, BET_AMOUNT * 2, "Contract should hold both bets");
// --- Action: Player A cancels the game AFTER Player B joined ---
uint256 playerABalanceBefore = playerA.balance;
uint256 playerBBalanceBefore = playerB.balance;
vm.expectEmit(true, false, false, true); // Expect GameCancelled event
emit GameCancelled(gameId);
vm.prank(playerA);
// This call succeeds due to the vulnerability
game.cancelGame(gameId);
// --- Assertions After Cancel ---
(,,,,,,,,,,,,,,, RockPaperScissors.GameState finalState) = game.games(gameId);
assertEq(uint256(finalState), uint256(RockPaperScissors.GameState.Cancelled), "Game state should be Cancelled");
// Verify both players were refunded (as per _cancelGame logic when playerB exists)
assertEq(playerA.balance, playerABalanceBefore + BET_AMOUNT, "Player A should be refunded");
assertEq(playerB.balance, playerBBalanceBefore + BET_AMOUNT, "Player B should be refunded");
assertEq(address(game).balance, 0, "Contract balance should be zero after refunds");
}

This test proves that Player A can invoke the cancellation logic after Player B has already joined and staked, exploiting the insufficient state check in cancelGame.

Impact

  • Griefing: Allows Player A to unfairly terminate a game after Player B has shown commitment by joining and staking. Player A might do this if they dislike the opponent, want to avoid playing, or simply to be disruptive.

  • Wasted Resources for Player B: Player B spends gas fees to approve token transfers (if applicable) and to call the joinGame... function, only to have their efforts nullified by Player A's cancellation. This wastes Player B's time and funds (gas).

  • Violation of Implicit Agreement: Once Player B joins, there's an implicit agreement for the game to proceed. Allowing Player A to unilaterally cancel at this stage breaks this agreement.

  • User Trust Erosion: This capability undermines trust in the fairness and reliability of the game platform, as players cannot be certain a game will proceed even after successfully joining.

(Note: This vulnerability does not lead to direct loss of staked ETH or Tokens for Player B, as _cancelGame handles the refund correctly. The impact is primarily related to griefing, wasted resources, and fairness.)

Tools Used

  • Manual Code Review

  • Foundry/Forge (for Test Execution and PoC verification)

Recommendations

To prevent Player A from cancelling after Player B has joined, the cancelGame function needs an additional check to ensure Player B has not yet joined.

  1. Add Player B Occupancy Check: Modify the cancelGame function to include a check for game.playerB.

    function cancelGame(uint256 _gameId) external {
    Game storage game = games[_gameId];
    require(game.state == GameState.Created, "Game must be in created state");
    require(msg.sender == game.playerA, "Only creator can cancel");
    + require(game.playerB == address(0), "Cannot cancel after player B joined"); // Add this check
    _cancelGame(_gameId);
    }

This additional requirement ensures that cancelGame can only be called by Player A before any opponent has successfully joined, aligning the function with its likely intended purpose and preventing the griefing vector.

Updates

Appeal created

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

Invalid TimeoutReveal Logic Error

timeoutReveal function incorrectly allows execution and game cancellation even when only one player has committed

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

Invalid TimeoutReveal Logic Error

timeoutReveal function incorrectly allows execution and game cancellation even when only one player has committed

Support

FAQs

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