Rock Paper Scissors

Summary

The RockPaperScissors.sol contract facilitates ETH-based games where funds are held in escrow and returned to players upon game completion (win, tie, or cancellation). The refund logic within the internal functions _handleTie and _cancelGame attempts to transfer ETH back to both players involved. However, these functions employ strict checks (require(successA && successB) or sequential require(successB)) that mandate the success of ETH transfers to all relevant parties within the same transaction. If one player (Player B) joins using a smart contract specifically designed to reject incoming Ether (lacking a payable receive/fallback or explicitly reverting), the ETH transfer to that player will fail. This failure causes the entire refund transaction to revert, consequently preventing the honest player (Player A) from receiving their refund as well. This creates a Denial of Service vulnerability, allowing a malicious actor to permanently lock both players' funds within the game contract.

Vulnerability Details

1. Refund Mechanism: The internal functions _handleTie (for tied games) and _cancelGame (for various cancellation scenarios) are responsible for returning staked ETH to players.

2. ETH Transfer Method: Both functions use the low-level .call{value: amount}("") method to send ETH refunds to game.playerA and, if applicable, game.playerB. This method returns a boolean success flag indicating the outcome of the transfer attempt.

3. Strict Success Requirement:

   • In _handleTie: After attempting transfers to both players, the code executes require(successA && successB, "Transfer failed");. This requires both transfers to have succeeded.

   • In _cancelGame: After attempting and requiring success for Player A's transfer, it attempts Player B's transfer (if playerB != address(0)) and then executes require(successB, "Transfer to player B failed");. This requires Player B's transfer to succeed if attempted.

4. Attack Vector: A malicious user can deploy a simple smart contract (RejectEthPlayer in the PoC) that lacks a payable receive() or payable fallback() function, or explicitly reverts on receiving ETH. This contract is then used to join a game as Player B.

5. Failure Point: When _handleTie or _cancelGame is triggered, the .call{value: ...} attempting to send ETH to the RejectEthPlayer contract address will fail, returning successB = false.

6. Transaction Revert: Due to the strict require(successA && successB, ...) or require(successB, ...) checks, the false value for successB causes the entire transaction to revert.

Proof Of Concept

The provided contract and test cases below demonstrate the vulnerability:

Impact

• Denial of Service (DoS): Honest users (Player A) are denied their rightful refunds when playing against a malicious contract designed to reject ETH.

• Permanent Fund Locking: Both Player A's and the malicious Player B's staked ETH become permanently locked within the RockPaperScissors contract, as the functions responsible for returning funds are blocked from successful execution.

• Griefing: Malicious actors can intentionally deploy and use such contracts to trap opponents' funds, rendering the game unusable for ETH refunds in tie/cancel scenarios.

• User Trust Erosion: This vulnerability breaks the core expectation that staked funds will be returned under tie or cancellation conditions, severely damaging user trust.

Tools Used

• Manual Code Review

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

Recommendations

The primary recommendation is to refactor the refund logic to avoid relying on the success of push payments (.call) within a single atomic transaction that affects multiple parties. The Pull Payment Pattern is the standard mitigation for this type of vulnerability.

1. Implement Pull Payments (Recommended):

   • Modify _handleTie and _cancelGame: Instead of attempting direct ETH transfers, update internal mappings to record the amount owed to each player (e.g., mapping(address => uint256) public refunds;).

   • Add a new external function like withdrawRefund().

   • Inside withdrawRefund(), a player (msg.sender) can call this function. It checks the refunds mapping for an amount owed to the caller, transfers the ETH using .call{value: amount}(""), requires success, and then sets the refund amount for that player back to zero in the mapping to prevent re-withdrawal.

   • Benefit: This isolates transfer failures. If Player B (the malicious contract) fails to withdraw or cannot receive ETH, it does not prevent Player A from successfully calling withdrawRefund() to claim their funds.

2. Remove Strict Cross-Party Success Checks (Less Ideal):

   • Alternative (Not Recommended): Remove the require(successA && successB) check from _handleTie and potentially the require(successB) from _cancelGame.

   • Drawback: While this would prevent the transaction from reverting due to Player B's failure and allow Player A's transfer to potentially succeed within that transaction, it doesn't solve the underlying problem that Player B's funds might still be locked if they genuinely cannot receive ETH. It also hides information about transfer failures. The pull payment pattern is significantly more robust and preferred.

Implementing the pull payment pattern will effectively mitigate this Denial of Service vulnerability and prevent the permanent locking of funds due to unpayable recipients.