neko_nyaa - `rescueERC20()` does not rescue stream tokens, however it is easily possible to support such functionality.

[sherlock-admin]

neko_nyaa

medium

rescueERC20() does not rescue stream tokens, however it is easily possible to support such functionality.

Summary

The current Stream.rescueERC20() will revert if the token to recover is the stream token. While this is stated in the docs, it is easy to support such functionality.

Vulnerability Detail

The invariant throughout a stream is that remainingBalance is the total amount left that the recipient is entitled to. Therefore if the token to recover is the stream token, the maximum amount to recover is just the balance of the stream itself, minus remainingBalance.

Impact

The impact is considered through the intention of the function itself: to provide a recovery for erroneous transferring.

With the main rescue function not working for stream tokens, said token can only be rescued by the cancel() method. Thereby the team will have to choose between cancelling the stream entirely, or to wait for an arbitrary long stream to end before recovering the funds.

Given that the stream token is currently not allowed for recovery, but is exactly the one to be interacted most with related to transferring to the contract, I believe the effect should be considered.

Code Snippet

https://github.com/sherlock-audit/2022-11-nounsdao/blob/main/src/Stream.sol#L268-L272

Tool used

Manual Review, VS code

Recommendation

Add appropriate recoverable amount calculation for the rescueERC20() function. One implementation is as such:

function rescueERC20(address tokenAddress, uint256 amount) external onlyPayer {
    if (tokenAddress == address(token())) {
        if (amount + remainingBalance > tokenBalance) {
            revert CannotRescueStreamToken();
        }
    } 

    IERC20(tokenAddress).safeTransfer(msg.sender, amount);
}

Duplicate of #28