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

Missing `commitMove` Deadline – If Player B fails to commit their move, the game becomes incomplete or non-executable, effectively rendering Player A’s committed move meaningless and the entire game unusable.

w33ked

Description

There is no deadline enforcement for commitMove can render a game unplayable when either playerA or playerB fails to commit their moves.
A reveal deadline is only set when both players have committed their moves.

// 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 means that playerA will never be able to reveal their moves based on game mechanics - game.revealDeadline has not been set.

The only option is to call timeoutReveal, which in turn calls _cancelGame, resulting in a refund for both players.

Impact

1. Unfair game theory
A players could chicken out of the game, leading to failure to commit moves, whereas the second player has committed their moves.

Proof of Concept

// test playerB failing to commit move after one year locking game
function testPlayerBFailingToCommitMove() public {
// player eth balances before
uint256 playerABalanceBefore = playerA.balance;
uint256 playerBBalanceBefore = playerB.balance;
​
​
gameId = createAndJoinGame();
​
​
// Player A commits
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);
​
// warp to reveal time
vm.warp(block.timestamp + TIMEOUT + 1);
​
// reveal
vm.expectRevert();
game.revealMove(gameId, uint8(RockPaperScissors.Move.Rock), saltA);
​
// player balances here
uint256 playerABalance = playerA.balance;
uint256 playerBBalance = playerB.balance;
​
vm.warp(block.timestamp + 365 days);
​
// check game state
(,,,,,,,,,,,,,,, RockPaperScissors.GameState state) = game.games(gameId);
assertEq(uint256(state), uint256(RockPaperScissors.GameState.Committed));
​
// playerA timeoutReveal
// calls _cancelGame
vm.prank(playerA);
game.timeoutReveal(gameId);
​
// player eth balances after
uint256 playerABalanceAfter = playerA.balance;
uint256 playerBBalanceAfter = playerB.balance;
​
// assert that balances are equal as before
assertEq(playerABalanceAfter, playerABalanceBefore);
assertEq(playerBBalanceAfter, playerBBalanceBefore);
}
​

Tools Used

Manual Code Review

Recommended mitigation

Add a commit moves deadline. This could be made to be the same amout or fime as timeouInterval

Updates

Appeal created

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

Player B cannot cancel a game if Player A becomes unresponsive after Player B joins

Protocol does not provide a way for Player B to exit a game and reclaim their stake if Player A stops participating

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

Player B cannot cancel a game if Player A becomes unresponsive after Player B joins

Protocol does not provide a way for Player B to exit a game and reclaim their stake if Player A stops participating

Support

FAQs

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