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

Give us feedback!