sherlock-audit / 2022-11-nounsdao-judging

4 stars 0 forks source link

dipp - Payer cannot recover overspent tokens sent to a stream without cancelling the stream #58

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

dipp

medium

Payer cannot recover overspent tokens sent to a stream without cancelling the stream

Summary

The payer of a stream could send more than tokenAmount of stream tokens to a stream. The overspent tokens are then unrecoverable by the payer without calling the cancel function in Stream.sol.

Vulnerability Detail

The rescueTokens function in Stream.sol does not allow the payer to transfer the stream's token out of the contract. Stream tokens may only be transferred out using the withdraw and cancel functions. If more than the tokenAmount of stream tokens are sent to the contract, the only way to recover these overspent tokens is to cancel the stream. The payer (or recipient) of the stream might not want to cancel the stream before the full amount has been vested, so the payer would be unable to recover their overspent tokens before all tokens have been vested by the user.

Impact

The payer is unable to retrieve overspent funds without cancelling the stream.

Code Snippet

[Stream.sol:withdraw#L213-L228]():

    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.
        unchecked {
            remainingBalance = remainingBalance - amount;
        }

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

[Stream.sol:cancel#L237-L259]():

    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);
    }

Stream.sol:rescueTokens#L268-L272:

    function rescueERC20(address tokenAddress, uint256 amount) external onlyPayer {
        if (tokenAddress == address(token())) revert CannotRescueStreamToken();

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

Tool used

Manual Review

Recommendation

Allow the payer to call rescueTokens to retrieve a max amount of tokenBalance - tokenAmount of stream tokens.

Duplicate of #28