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

`RockPaperScissors::_cancelGame` mints new tokens instead of refunding deposits, causing inflation and locked funds.

roccomania

Summary

The internal _cancelGame function incorrectly processes refunds for cancelled token-based games. Instead of returning the WinningTokens previously deposited by the players, it mints new tokens for them, leaving the original deposits locked within the contract.

Vulnerability Details

For token-based games (game.bet == 0), the standard flow involves Player A depositing 1 WinningToken via createGameWithToken. If Player B joins using joinGameWithToken, they also deposit 1 token. Thus, the contract holds 1 or 2 tokens depending on whether Player B joined before cancellation.

If the game is cancelled (e.g., via timeoutJoin or timeoutReveal leading to cancellation), the _cancelGame function is called to handle refunds. The logic for token games within this function is:

// In RockPaperScissors.sol, _cancelGame function
​
} else {
// Token game - return tokens to players
if (game.playerA != address(0)) {
winningToken.mint(game.playerA, 1); // <-- Mints 1 NEW token for Player A
}
if (game.playerB != address(0)) {
// If player B joined, they also deposited a token
winningToken.mint(game.playerB, 1); // <-- Mints 1 NEW token for Player B
}
emit GameCancelled(_gameId);
}
​

The winningToken.mint(...) calls create new WinningTokens for the players involved. This mechanism fails to return the actual token(s) that were deposited into the contract when the game was created (and potentially joined). These original tokens remain held by the RockPaperScissors contract address.

Impact

This vulnerability mirrors the issues found in _finishGame and _handleTie, specifically for cancelled games:

  • Locked Tokens:
    The 1 or 2 WinningTokens deposited for the cancelled game become permanently locked within the RockPaperScissors contract. There is no way to retrieve these specific tokens later.

  • Unnecessary Token Inflation:
    The WinningToken supply increases by 1 or 2 tokens (depending on whether Player B joined) for every cancelled token game. This inflates the supply unnecessarily.

  • Incorrect Refund Mechanism:
    Players expect their original deposit back when a game is cancelled. Receiving a newly minted token is not the correct way to handle a refund of staked assets.

Tools Used

Manual code review.

Recommendations

The _cancelGame function should be updated to transfer the existing tokens held by the contract back to the players involved, instead of minting new ones.

Replace the mint calls with transfer calls:

} else {
// Token game - return tokens to players
uint256 tokensToRefund = 0;
if (game.playerA != address(0)) {
- winningToken.mint(game.playerA, 1);
+ // Mark that Player A needs a refund
+ tokensToRefund++;
}
if (game.playerB != address(0)) {
// If player B joined, they also deposited a token
- winningToken.mint(game.playerB, 1);
+ // Mark that Player B needs a refund
+ tokensToRefund++;
}
​
+ // Ensure the contract has enough tokens for the refund
+ require(winningToken.balanceOf(address(this)) >= tokensToRefund, "Contract token balance insufficient for cancel refund"); // Safety check
+
+ // Perform the transfers
+ if (game.playerA != address(0)) {
+ winningToken.transfer(game.playerA, 1);
+ }
+ if (game.playerB != address(0)) {
+ winningToken.transfer(game.playerB, 1);
+ }
+
emit GameCancelled(_gameId);
}
​
Updates

Appeal created

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

Minting Instead of Transferring Staked Tokens

Mints new tokens upon game completion or cancellation for token-based games

Support

FAQs

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