Rock Paper Scissors
First Flight #38
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Incorrect State Management During Commit/Reveal Phases

howiecht

Summary

The contract does not properly transition between distinct game states for the commit and reveal phases, overloading the GameState.Committed state. This makes the contract logic less robust and harder to reason about.

Vulnerability Details

The game transitions to GameState.Committed when the first player commits (if player B has joined). It remains in GameState.Committed even after both players have committed and the revealDeadline is set. The revealMove and timeoutReveal functions correctly require that the deadline is active (revealDeadline is set) but incorrectly check for the GameState.Committed state. There should be a distinct state indicating the reveal phase is active.

Impact

  • Reduced Code Clarity: Overloading the Committed state makes the contract's control flow harder to understand and audit.

  • Potential for Future Bugs: If the contract logic around commits, reveals, or deadlines is modified later, the lack of a distinct reveal state could more easily lead to vulnerabilities or incorrect game behavior.

Tools Used

Recommendations

Fix and Add correct reveal state.

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;
// change to phase into GameState.Revealed
+ game.state = GameState.Revealed;
}
}
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"); // @audit need into Revealed Phase
+ 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");
​
// ...
}
function timeoutReveal(uint256 _gameId) 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"); // @audit in the GameState.Revealed
+ require(game.state == GameState.Committed, "Game not in reveal phase");
require(block.timestamp > game.revealDeadline, "Reveal phase not timed out yet");
​
// ...
}
Updates

Appeal created

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