sherlock-audit / 2022-11-nounsdao-judging

4 stars 0 forks source link

KingNFT - The ````Stream```` contract is designed to receive ETH but not implement function for withdrawal #47

Open sherlock-admin opened 1 year ago

sherlock-admin commented 1 year ago

KingNFT

medium

The Stream contract is designed to receive ETH but not implement function for withdrawal

Summary

The Stream contract instances can receive ETH but can not withdraw, ETH occasionally sent by users will be stuck in those contracts.

Vulnerability Detail

Shown as the test case, it can receive ETH normally.

contract StreamReceiveETHTest is StreamTest {
    function setUp() public override {
        super.setUp();
    }

    function test_receiveETH() public {
        s = Stream(
            factory.createStream(
                payer, recipient, STREAM_AMOUNT, address(token), startTime, stopTime
            )
        );

        vm.deal(payer, 10 ether);
        vm.prank(payer);
        (bool success, ) = address(s).call{value: 1 ether}("");
        assertEq(success, true);
        assertEq(address(s).balance, 1 ether);
    }
}

Result

Running 1 test for test/Stream.t.sol:StreamReceiveETHTest
[PASS] test_receiveETH() (gas: 167691)
Test result: ok. 1 passed; 0 failed; finished in 1.25ms

Impact

See Summary

Code Snippet

https://github.com/Vectorized/solady/blob/db4857b4a1e17ad035668b588b41a1c90139b99d/src/utils/LibClone.sol#L193-L204

Tool used

Manual Review

Recommendation

Add a rescueETH() function which is similar with the existing rescueERC20()

eladmallel commented 1 year ago

Fix PR: https://github.com/nounsDAO/streamer/pull/10

jack-the-pug commented 1 year ago

Fix confirmed!