0xZakk - Only way to retrieve over sent funds is to cancel the stream

[sherlock-admin]

0xZakk

medium

Only way to retrieve over sent funds is to cancel the stream

Summary

If you over send funds to the contract (send more than the tokenAmount), there's no way to retrieve the difference other than to cancel the entire stream.

Vulnerability Detail

    /**
     * @notice Withdraw tokens to recipient's account.
     * Execution fails if the requested amount is greater than recipient's withdrawable balance.
     * Only this stream's payer or recipient can call this function.
     * @param amount the amount of tokens to withdraw.
     */
    function withdraw(uint256 amount) external onlyPayerOrRecipient {
        if (amount == 0) revert CantWithdrawZero();
        address recipient_ = recipient();

        uint256 balance = balanceOf(recipient_);
        if (balance < amount) revert AmountExceedsBalance();

        // This is safe because it should always be the case that:
        // remainingBalance >= balance >= amount.
        // @audit-ok - test this assumption
        unchecked {
            remainingBalance = remainingBalance - amount;
        }

        token().safeTransfer(recipient_, amount);
        emit TokensWithdrawn(msg.sender, recipient_, amount);
    }

    /**
     * @notice Cancel the stream and send payer and recipient their fair share of the funds.
     * If the stream is sufficiently funded to pay recipient, execution will always succeed.
     * Payer receives the stream's token balance after paying recipient, which is fair if payer
     * hadn't fully funded the stream.
     * Only this stream's payer or recipient can call this function.
     */
    // @audit - if you over send you have to cancel the stream to get the remainder back
    function cancel() external onlyPayerOrRecipient {
        address payer_ = payer();
        address recipient_ = recipient();
        IERC20 token_ = token();

        uint256 recipientBalance = balanceOf(recipient_);

        // This zeroing is important because without it, it's possible for recipient to obtain additional funds
        // from this contract if anyone (e.g. payer) sends it tokens after cancellation.
        // Thanks to this state update, `balanceOf(recipient_)` will only return zero in future calls.
        remainingBalance = 0;

        if (recipientBalance > 0) token_.safeTransfer(recipient_, recipientBalance);

        // Using the stream's token balance rather than any other calculated field because it gracefully
        // supports cancelling the stream even if payer hasn't fully funded it.
        uint256 payerBalance = tokenBalance();
        if (payerBalance > 0) {
            token_.safeTransfer(payer_, payerBalance);
        }

        emit StreamCancelled(msg.sender, payer_, recipient_, payerBalance, recipientBalance);
    }

There's no way to withdraw any surplus tokens sent over the tokenAmount. The only way to extra funds back would be to wait for the stopTime to pass and the tokens to be withdrawn and then call cancel or to cancel the stream before stopTime and create an entirely new stream.

Impact

If creating a new stream requires a governance proposal, surplus tokens are locked until stopTime has elapsed.

Code Snippet

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

Tool used

Manual Review

Recommendation

Update rescueERC20 to be able to rescue surplus tokens in excess of tokenAmount.

Duplicate of #28