Rock Paper Scissors

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

Reveal deadline reset issue

Summary

The reveal deadline attribute is not reset after turn 1 of the game, which allows an malicious person to frontrun turn 2, and calling timeoutReveal() to win the game.

Vulnerability Details

The vulnerability affects interactions between the following 4 functions: commitMove(), revealMove(), timeoutReveal() and _determineWinner().

The game must already be created and joined by a 2nd player.

The vulnerability appears at the end of the 1st round, when players have revealed their moves.

In the first round, there's no problem. The revealDeadline attribute has not yet been modified, so revealDeadline = 0. When both players have committed their action, this variable is set:

function commitMove(uint256 _gameId, bytes32 _commitHash) external {
...
// If both players have committed, set the reveal deadline
if (game.commitA != bytes32(0) && game.commitB != bytes32(0)) {
game.revealDeadline = block.timestamp + game.timeoutInterval;
}
}

This will be verified during the revelation phase:

function revealMove(uint256 _gameId, uint8 _move, bytes32 _salt) external {
...
require(
block.timestamp <= game.revealDeadline,
"Reveal phase timed out"
);
...
}

Once both players have revealed their game, the _determineWinner() function is called.

This function, as its name suggests, determines the winner, and resets the game attributes to move on to the next round if the game isn't over:

function _determineWinner(uint256 _gameId) internal {
...
// 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;
}

Here, the “revealDeadline” attribute is not reset, it is always set to :

game.revealDeadline = block.timestamp + game.timeoutInterval;

If the new block has not yet been added to the blockchain, it is possible to frontrun the 2nd turn of the game, adding the transactions of the following functions :

  • commitMove()

  • revealMove() → valid because block.timestamp + game.timeoutInterval > block.timestamp

If the 2nd player doesn't play before the end of the 1st game's allotted time, the attacker can call the timeoutReveal() function and recover the funds bet on this game.

Impact

This vulnerability is critical as it allows an attacker to steal the funds that the opposing player has staked.

Tools Used

Manual review

Recommendations

To counter this vulnerability, there are 2 solutions:

  • Reset the revealDeadline variable in the _determineWinner function:

function _determineWinner(uint256 _gameId) internal {
...
// 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;
}
  • Use GameState.Revealed instead of GameState.Committed :

function commitMove(uint256 _gameId, bytes32 _commitHash) external {
...
// If both players have committed, set the reveal deadline
if (game.commitA != bytes32(0) && game.commitB != bytes32(0)) {
game.revealDeadline = block.timestamp + game.timeoutInterval;
game.state = GameState.Revealed;
}
}
function revealMove(uint256 _gameId, uint8 _move, bytes32 _salt) external {
...
require(
block.timestamp <= game.revealDeadline,
"Reveal phase timed out"
);
require(game.state == GameState.Revealed, "Game not in reveal phase");
...
}
Updates

Appeal created

m3dython Lead Judge 2 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

xgrybto Submitter
2 months ago
m3dython Lead Judge
about 2 months ago
m3dython Lead Judge about 2 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.