Rock Paper Scissors

Summary

A malicious actor can create a contract to act as Player A with a gas-intensive fallback function that consumes nearly all available gas when receiving ETH. This can prevent Player B from successfully completing critical game actions that involve ETH transfers to Player A, potentially forcing Player B to lose by timeout.

Vulnerability Details

The RockPaperScissors contract uses low-level call to transfer ETH in several functions:

When ETH is transferred to an address, if that address is a contract, its fallback function (receive() or fallback()) is executed. A malicious contract can implement a gas-intensive fallback function that consumes nearly all available gas, causing subsequent operations to fail due to out-of-gas errors.

A malicious Player A could deploy a contract like this:

Impact

This vulnerability can be exploited in several scenarios:

1. Forced Timeout: Player A (malicious contract) commits and reveals their move, but when Player B tries to reveal their move, the transaction fails due to out-of-gas when _determineWinner is called and attempts to transfer ETH to Player A. Player B cannot complete their reveal before the deadline, allowing Player A to claim a win by timeout.

2. Blocked Timeout Claims: If Player B attempts to call timeoutReveal() when Player A has revealed but Player B hasn't, the function would try to send ETH to Player A, triggering the gas-consuming fallback function and causing the transaction to fail.

3. Prevented Game Cancellation: If Player B attempts to join a game and then Player A tries to cancel it, the _cancelGame() function would fail when trying to refund Player B, potentially locking funds.

This is a high severity issue because it can lead to direct fund loss for Player B through manipulation of the game mechanics, effectively allowing Player A to steal Player B's bet.

Tools Used

• Manual code review

• Analysis of gas consumption patterns

• Smart contract interaction simulation

Recommendations

Implement one of the following solutions:

1. Use the Pull Pattern Instead of Push: Instead of directly sending ETH to winners, implement a withdrawal pattern where players can claim their winnings:

   // Add a mapping to track claimable amounts

   mapping(address => uint256) public claimableAmount;

   ​

   // Update _finishGame to record the winnings instead of sending them

   function _finishGame(uint256 _gameId, address _winner) internal {

   Game storage game = games[_gameId];

   game.state = GameState.Finished;

   if (game.bet > 0) {

   uint256 totalPot = game.bet * 2;

   uint256 fee = (totalPot * PROTOCOL_FEE_PERCENT) / 100;

   uint256 prize = totalPot - fee;

   // Record the prize instead of sending it

   claimableAmount[_winner] += prize;

   accumulatedFees += fee;

   }

   // ... rest of the function ...

   }

   ​

   // Add a function for winners to claim their prizes

   function claimPrize() external {

   uint256 amount = claimableAmount[msg.sender];

   require(amount > 0, "No prize to claim");

   claimableAmount[msg.sender] = 0;

   (bool success,) = msg.sender.call{value: amount}("");

   require(success, "Transfer failed");

   }

2. Set a Gas Limit for ETH Transfers: If you must use direct transfers, set a gas limit to prevent malicious contracts from consuming all gas:

   // Instead of:

   (bool success,) = _winner