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 10 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 10 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.

Give us feedback!