Rock Paper Scissors

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

User ETH will Stuck in the contract permenently because there is no f=mechanism to withdraw contract balance.

Description

There is `receive` function which allows user to send ETH directly to the contract. But there is no functionality or mechanism to withdraw that ETH results into permenetly stuck into the contract even if that ETH was donated to the game no one can withdraw it from the contract.

Impact

The funds will stuck in contract which is not withrawable to anyone include contract owner itself.

Proof of Concept

Add this test into the test file.
```javascript
function test_userSentETHDirectlyToTheContractAndAdminCannotWithdrawIt() public {
vm.prank(playerA);
(bool ok,) = address(game).call{value: 1e18}("");
assert(ok);
assertEq(address(game).balance, 1e18);
vm.prank(admin);
vm.expectRevert();
game.withdrawFees(1e18);
console.log("Balance of the contract:", address(game).balance);
}
```
Result:
```javascript
[PASS] test_userSentETHDirectlyToTheContractAndAdminCannotWithdrawIt() (gas: 29528)
Logs:
Balance of the contract: 1000000000000000000
Traces:
[29528] RockPaperScissorsTest::test_userSentETHDirectlyToTheContractAndAdminCannotWithdrawIt()
├─ [0] VM::prank(playerA: [0x23223AC37AC99a1eC831d3B096dFE9ba061571CF])
│ └─ ← [Return]
├─ [80] RockPaperScissors::receive{value: 1000000000000000000}()
│ └─ ← [Stop]
├─ [0] VM::assertEq(1000000000000000000 [1e18], 1000000000000000000 [1e18]) [staticcall]
│ └─ ← [Return]
├─ [0] VM::prank(RockPaperScissorsTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496])
│ └─ ← [Return]
├─ [0] VM::expectRevert(custom error 0xf4844814)
│ └─ ← [Return]
├─ [4775] RockPaperScissors::withdrawFees(1000000000000000000 [1e18])
│ └─ ← [Revert] revert: Insufficient fee balance
├─ [0] console::log("Balance of the contract:", 1000000000000000000 [1e18]) [staticcall]
│ └─ ← [Stop]
└─ ← [Return]
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 4.77ms (2.29ms CPU time)
```

Recommendations

The protocol should remove the `receive` fallback function from the contract.
```diff
- receive() external payable {
- // Allow contract to receive ETH
- }
```
Or implement the machanism so that admin or owner of the game can withdraw that directly transfered funds.
Updates

Appeal created

m3dython Lead Judge about 1 month 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.