Rock Paper Scissors

Summary

The RockPaperScissors.sol contract implements a Rock Paper Scissors game allowing bets in ETH or a specific ERC20 token (WinningToken). While the contract correctly takes custody of WinningToken stakes via transferFrom when players create or join token-based games, it fails to return these specific tokens upon game completion (win, tie, or cancellation). Instead, it attempts to mint new tokens for the players using winningToken.mint(). This results in the original staked tokens being permanently locked within the RockPaperScissors contract balance, leading to a direct loss of assets for users participating in token-based games.

Vulnerability Details

1. Staking Mechanism: When a player initiates or joins a token-based game, the createGameWithToken and joinGameWithToken functions correctly use winningToken.transferFrom(player, address(this), 1). This transfers ownership of one WinningToken from the player to the RockPaperScissors contract (address(this)), placing the token under the contract's custody as the game stake.

2. Payout/Refund Mechanism: Upon game conclusion for token-based games (game.bet == 0), the internal functions _finishGame (for a win), _handleTie (for a tie), and _cancelGame (for cancellation) are responsible for returning the stakes.

3. Incorrect Function Call: Instead of transferring the tokens held by the contract back to the relevant player(s) using winningToken.transfer(player, amount), these functions incorrectly call winningToken.mint(player, amount).

   • _finishGame: Calls winningToken.mint(_winner, 2).

   • _handleTie: Calls winningToken.mint(game.playerA, 1) and winningToken.mint(game.playerB, 1).

   • _cancelGame: Calls winningToken.mint(player, 1) for existing players.

4. Discrepancy: The mint function creates entirely new tokens and assigns them to the recipient. It does not interact with or reduce the existing token balance held by the RockPaperScissors contract.

5. Consequence: The original tokens transferred to the RockPaperScissors contract during the staking phase are never transferred out. They remain in the contract's balance indefinitely with no function available to withdraw them.

Proof Of Concept

The test cases below demonstrates the flaw.

Impact

• Permanent Token Lock: The original tokens staked by the players remain permanently locked in the RockPaperScissors contract's balance with no mechanism for recovery.

• Direct Financial Loss: Users participating in token-based games (game.bet == 0) will permanently lose the WinningToken they staked to create or join the game. The tokens are irrecoverable as there is no withdrawal mechanism for them.

• Incorrect Token Supply Inflation: The total supply of WinningToken increases unexpectedly every time a token-based game concludes (win, tie, or cancel), as new tokens are minted instead of existing ones being circulated. This can disrupt the token's intended economics.

• User Trust Erosion: Discovering that staked assets are not returned correctly will severely damage user trust in the application.

Tools Used

• Manual Code Review

• Foundry/Forge (for Test Execution and PoC verification)

Recommendations

The core recommendation is to replace the incorrect mint calls with transfer calls when handling token payouts or refunds for token-based games (game.bet == 0).

1. Modify _finishGame:

   • Replace winningToken.mint(_winner, 2);

   • With: winningToken.transfer(_winner, 2);

2. Modify _handleTie:

   • Replace winningToken.mint(game.playerA, 1); and winningToken.mint(game.playerB, 1);

   • With: winningToken.transfer(game.playerA, 1); and winningToken.transfer(game.playerB, 1);

3. Modify _cancelGame:

   • Replace winningToken.mint(game.playerA, 1); (inside the if)

   • With: winningToken.transfer(game.playerA, 1);

   • Replace winningToken.mint(game.playerB, 1); (inside the if)

   • With: winningToken.transfer(game.playerB, 1);

4. Consider SafeERC20: For enhanced robustness, consider using OpenZeppelin's SafeERC20 library and its safeTransfer function instead of the standard transfer. This protects against potential issues with certain ERC20 token implementations that might not return booleans or could behave unexpectedly.

   // Example using SafeERC20

   import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

   // ... inside contract ...

   using SafeERC20 for WinningToken;

   // ... inside _handleTie ...

   token.safeTransfer(game.playerA, 1);

   token.safeTransfer(game.playerB, 1);

By implementing these changes, the contract will correctly transfer the tokens it holds back to the players, resolving the locked tokens vulnerability.