Rock Paper Scissors
First Flight #38
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Valid

Loss of native eth funds send to `RockPaperScissors`

lucky2892000

[H-3] summary

Loss of native eth funds send to RockPaperScissors

vulnerability details

In our RockPaperScissors contract we have the payable receive function which accepts direct eth transfer

receive() external payable {
// Allow contract to receive ETH
}

But we can't able to recover those funds and those will be lost forever

POC

-> send Eth the RockPaperScissors contract
-> RockPaperScissors contract only has withdrawFees() function where owner able to withdraw the on the accumulatedFees and not more than that
-> There is no other function provided to recover the native eth sent to the contract

impact - High

likelyhood - Medium

Recommendations

  1. we can add the below function to retrive the funds that was sent to the contract
    added condition with RockPaperScissors::accumulatedFees so that we wont withdraw the funds that was accumulated through the protocol fees which we use RockPaperScissors::withdrawFees to withdraw

function recoverFunds() external {
require(msg.sender == adminAddress, "Only admin can recover the funds");
require(address(this).balance > accumulatedFees, "No extra ETH was sent to this contract");
(bool success,) = adminAddress.call{value: address(this).balance - accumulatedFees }("");
require(success, "Recovery failed");
}

  1. If we thought of no direct eth was needed , then we can remoe the payable receive funtion from the contract , so there will be no funds to lost as it will revert on direct eth transfers

Updates

Appeal created

m3dython Lead Judge about 2 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
m3dython Lead Judge about 2 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
lucky2892000 Submitter
about 2 months ago
m3dython Lead Judge
about 2 months ago
lucky2892000 Submitter
about 2 months ago
m3dython Lead Judge about 2 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.