Rock Paper Scissors

Summary

The protocol's ETH refund mechanism in tie scenarios uses an unsafe "all-or-nothing" transfer approach that risks permanently locking user funds if either refund transfer fails. This violates the atomicity principle and lacks recovery mechanisms for partial failures.

Vulnerability Details

Affected Function: _handleTie()

Root Cause:

The contract attempts to send ETH refunds to both players in a single atomic transaction:

If either transfer fails (even if one succeeds temporarily), the entire transaction reverts, leaving both refunds unprocessed and ETH permanently stuck in the contract.

Failure Scenarios:

1. Malicious Contracts: A player contract rejects ETH transfers in its receive() function

2. Gas Limits: Complex recipient logic exhausts gas during transfer

3. Temporary Issues: Recipient addresses with intermittent failure states

Impact

Severity: High
Likelihood: Medium (Depends on user address types)
Consequences:

• Permanent loss of refund ETH for both players

• Protocol accumulates locked funds with no recovery path

• Secondary attacks possible by griefers forcing failures

PoC

Foundry Test Case:

Tools Used

1. Manual Review: Identified unsafe atomic transfer pattern

2. Foundry: Executed PoC with contract-based griefing attack

Recommendations

1. Implement Pull Payments:

   mapping(address => uint256) public pendingWithdrawals;

   ​

   function withdraw() external {

   uint256 amount = pendingWithdrawals[msg.sender];

   require(amount > 0, "No funds pending");

   pendingWithdrawals[msg.sender] = 0;

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

   require(success, "Transfer failed");

   }

   ​

   // In _handleTie():

   pendingWithdrawals[playerA] += refundA;

   pendingWithdrawals[playerB] += refundB;

2. Add Emergency Recovery:

   function adminRecoverETH(address recipient) external onlyAdmin {

   (bool success,) = recipient.call{value: address(this).balance}("");

   require(success, "Transfer failed");

   }

3. Circuit Breaker:

   bool public withdrawalsPaused;

   ​

   function withdraw() external {

   require(!withdrawalsPaused, "Withdrawals paused");

   // ... existing logic ...

   }

This approach decouples accounting from fund transfers, eliminates atomic failure risks, and provides admin safeguards while preserving user control over withdrawals.