Rock Paper Scissors

Description

The RockPaperScissors contract uses low-level call{value: ...}("") to transfer ETH to game winners, both players in a tie, and the admin (for fee withdrawal). Each transfer is immediately followed by a require(success, ...) statement. If the recipient is a contract that reverts in its fallback/receive function (either maliciously or accidentally), the entire transaction reverts and the contract state remains unchanged. This permanently locks the game’s prize pool or protocol fees, as there is no alternative withdrawal mechanism.

Summary

• Affected Functions: _finishGame, _handleTie, _cancelGame, withdrawFees

• Root Cause: Use of push-payment (call{value: ...}("")) with require(success) to external addresses.

• Impact: Any recipient can block the payout or refund, causing permanent loss of funds for all parties involved.

Vulnerability Details

1. Winner Payout

If _winner is a contract that reverts, the game cannot finish and the prize is locked forever.

2. Tie Refunds

If either player reverts, both refunds fail and the pot is locked.

3. Admin Fee Withdrawal

If the admin address is a contract that reverts, all protocol fees become permanently inaccessible.

Impact

• Permanent loss of user funds: Players’ stakes can be locked forever if a malicious or buggy contract is the recipient.

• Permanent loss of protocol fees: Admin cannot withdraw fees if the admin address is a contract that reverts.

• No recovery: There is no alternative “pull” mechanism or admin escape hatch.

Tools Used

Manual code review.

Recommendations

1. Adopt a pull-payment pattern:

   • Store owed balances in a mapping and allow users/admin to withdraw via a separate function.

   • Use OpenZeppelin’s PullPayment or Escrow helpers.

2. If push-payments are retained:

   • Remove require(success) and, on failure, credit the amount to a withdrawable balance.

   • Never couple multiple transfers with a single require(successA && successB); handle each transfer independently.

3. Add an admin escape hatch:

   • Allow the owner or a multisig to recover stuck funds after a grace period.