Rock Paper Scissors

First Flight #38
Beginner FriendlySolidity
100 EXP
View results
Submission Details
Severity: medium
Valid

Funds received thru receive() are locked🔒

Summary

The Rock Paper Scissors contract includes a receive() function that allows it to accept ETH directly, but provides no mechanism to withdraw this ETH. This creates a permanent ETH lock situation where funds sent directly to the contract become irretrievable.

Vulnerability Details

The contract includes a receive() function that accepts ETH without any restrictions:

/**
* @dev Fallback function to accept ETH
*/
receive() external payable {
// Allow contract to receive ETH
}

However, the only ETH withdrawal mechanism in the contract is the withdrawFees() function, which only allows the admin to withdraw from the accumulatedFees variable:

function withdrawFees(uint256 _amount) external {
require(msg.sender == adminAddress, "Only admin can withdraw fees");
​
uint256 amountToWithdraw = _amount == 0 ? accumulatedFees : _amount;
require(amountToWithdraw <= accumulatedFees, "Insufficient fee balance");
​
accumulatedFees -= amountToWithdraw;
​
(bool success,) = adminAddress.call{value: amountToWithdraw}("");
require(success, "Fee withdrawal failed");
​
emit FeeWithdrawn(adminAddress, amountToWithdraw);
}

The problem is that ETH sent through the receive() function:

  1. Is added to the contract's general balance

  2. Is not tracked in the accumulatedFees variable

  3. Cannot be withdrawn through the withdrawFees() function

  4. Has no other withdrawal mechanism

Impact

This vulnerability has medium impact because:

  1. Any ETH sent directly to the contract (not through game functions) is permanently locked

  2. Users or admins who accidentally send ETH to the contract address will lose those funds

  3. There is no recovery mechanism for these locked funds

This issue is compounded because having a receive() function explicitly signals that the contract is designed to accept ETH, yet that ETH becomes trapped.

Tools Used

Manual code review

Recommendations

There are two possible approaches to fix this vulnerability:

Option 1: Remove the receive function

// Remove this function entirely
// receive() external payable {
// // Allow contract to receive ETH
// }

This prevents ETH from being sent directly to the contract, making the contract behavior clearer.

Option 2: Add a withdrawal mechanism for all contract balance

Either approach would prevent ETH from being permanently locked in the contract.

Updates

Appeal created

m3dython Lead Judge 4 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Orphaned ETH due to Unrestricted receive() or Canceled Game

ETH sent directly to the contract via the receive function or after a canceled game becomes permanently locked

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.