Rock Paper Scissors

Summary

The RockPaperScissors contract is vulnerable to a cross-function reentrancy attack through the timeoutReveal function. This vulnerability allows a malicious player to drain funds from multiple games by exploiting the lack of reentrancy protection when ETH is transferred to winners.

Vulnerability Details

The vulnerability exists in the interaction between the timeoutReveal function and the internal _finishGame function it calls:

1. In timeoutReveal (lines 263-286), when a player has revealed their move but their opponent hasn't before the deadline, the function calls _finishGame with the revealing player as the winner.

2. In _finishGame (lines 473-506), the contract:

   • Sets the game state to Finished (line 476)

   • Calculates the prize (lines 482-485)

   • Updates the accumulated fees (line 488)

   • Sends ETH to the winner using a low-level call (line 492):

     (bool success,) = _winner.call{value: prize}("");

3. The low-level call can trigger a fallback function in a malicious contract, allowing it to call back into the RockPaperScissors contract before the original transaction completes.

4. While the game state for the current game is set to Finished before the ETH transfer (which prevents reentrancy on the same game), there's no global lock to prevent reentrancy across different games or functions.

Attack Scenario:

1. Attacker creates 4 games with ETH bets using a malicious contract

2. Attacker joins these games with another account

3. Attacker legitimately wins Game #1 by timeout (reveals move while malicious contract doesn't)

4. When receiving ETH from Game #1, attacker reenters and calls timeoutReveal() on Games #2, #3, and #4

5. Attacker steals funds from all games in a single transaction, even though they should only win Game #1

Sample attack contract:

Impact

This vulnerability has a high severity impact as it allows a malicious player to:

1. Steal all ETH from games they've participated in

2. Drain the contract of funds meant for other players

3. Potentially manipulate game outcomes unfairly

The financial impact is directly proportional to the number and size of active games with ETH bets. In a production environment with multiple games and significant betting amounts, this could lead to substantial financial losses.

Tools Used

• Manual code review

Recommendations

To fix this vulnerability, implement the following changes:

1. Add a reentrancy guard using OpenZeppelin's ReentrancyGuard:

1. Alternatively, implement a custom reentrancy guard:

1. Follow the checks-effects-interactions pattern more strictly by:

   • Performing all state changes before external calls

   • Using the pull-over-push pattern for prize distribution

   • Consider implementing a separate withdrawal function for winners to claim prizes

2. Consider using OpenZeppelin's SafeERC20 for token transfers and Address library for safe ETH transfers.