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

Game cancellation in multi turn games overwrites previous scores

ciphermalware

Summary

In multi-turn games, if both players fail to reveal their moves within the revealDeadline for any turn, the timeoutReveal function allows either player to call _cancelGame, which cancels the entire game and refunds the bets (ETH or tokens). This behavior discards all previous turns’ scores allowing a losing player to force a cancellation and recover their bet instead of conceding a loss.

Vulnerability Details

The affected code is the following:

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");
require(block.timestamp > game.revealDeadline, "Reveal phase not timed out yet");
​
// If player calling timeout has revealed but opponent hasn't, they win
bool playerARevealed = game.moveA != Move.None;
bool playerBRevealed = game.moveB != Move.None;
​
if (msg.sender == game.playerA && playerARevealed && !playerBRevealed) {
// Player A wins by timeout
_finishGame(_gameId, game.playerA);
} else if (msg.sender == game.playerB && playerBRevealed && !playerARevealed) {
// Player B wins by timeout
_finishGame(_gameId, game.playerB);
} else if (!playerARevealed && !playerBRevealed) {
// Neither player revealed, cancel the game and refund
_cancelGame(_gameId);
} else {
revert("Invalid timeout claim");
}
}

Impact

This undermines the fairness of multi turn games by allowing a losing player to mitigate their loss, though it requires specific conditions such as both players not revealing.

Recommendations

I would recommend to modify timeoutReveal to award the win based on current scores if some turns have been completed, rather than cancelling the game outright, like so:

if (!playerARevealed && !playerBRevealed) {
if (game.currentTurn > 1 && (game.scoreA > game.scoreB || game.scoreB > game.scoreA)) {
address winner = game.scoreA > game.scoreB ? game.playerA : game.playerB;
_finishGame(_gameId, winner);
} else {
_cancelGame(_gameId); // Cancel only if no turns completed or scores tied
}
}
Updates

Appeal created

m3dython Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Design choice
m3dython Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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