Rock Paper Scissors

Summary

The contract performs Ether transfers to user-controlled addresses using the low-level .call{value:} method before updating internal state. This violates the Checks-Effects-Interactions pattern, potentially exposing the contract to reentrancy attacks if playerA or playerB is a malicious smart contract.

Vulnerability Details

The vulnerable code is:

if (game.bet > 0) {
(bool successA,) = game.playerA.call{value: game.bet}("");
require(successA, "Transfer to player A failed");

• Since internal state (e.g., game.bet) is not updated before this call, a malicious player contract can re-enter the function before state changes occur, potentially triggering double withdrawals or logic manipulation.

• game.playerA.call{value: game.bet}("") allows execution of arbitrary logic in playerA's fallback or receive function.

Impact

• Reentrancy Attack Vector: Malicious contracts can recursively call back into the vulnerable function.

• Fund Drain Risk: If internal balances or bet values aren’t reset prior to transfers, attackers could drain funds.

• Contract Inconsistency: Unfinalized game state could be manipulated mid-execution.

Tools Used

Manual review

Recommendations

• Use ReentrancyGuard: Apply OpenZeppelin’s ReentrancyGuard and mark function with nonReentrant.

• Pull Payments Pattern: Refactor contract to let players withdraw funds themselves via a separate withdraw() function.

• Whitelist Addresses (Optional): Consider restricting payouts to EOAs or vetted contracts if game design allows.

• Follow Checks-Effects-Interactions Pattern: Move all state updates (e.g., resetting game.bet) before external calls.