Summary

https://github.com/Cyfrin/2024-05-Sablier/blob/43d7e752a68bba2a1d73d3d6466c3059079ed0c6/v2-core/test/mocks/erc20/ERC20MissingReturn.sol#L66-L70

https://github.com/Cyfrin/2024-05-Sablier/blob/43d7e752a68bba2a1d73d3d6466c3059079ed0c6/v2-core/src/abstracts/SablierV2Lockup.sol#L410-L433

The function withdrawMaxAndTransfer withdraws and transfers streamID and it calls the internal transfer function which transfers amount instead of the streamID.

Vulnerability Details

`function withdrawMaxAndTransfer(
    uint256 streamId,
    address newRecipient
)
    external
    override
    noDelegateCall
    notNull(streamId)
{
    // Check: the caller is the current recipient. This also checks that the NFT was not burned.
    address currentRecipient = _ownerOf(streamId);
    if (msg.sender != currentRecipient) {
        revert Errors.SablierV2Lockup_Unauthorized(streamId, msg.sender);
    }


    // Skip the withdrawal if the withdrawable amount is zero.
    uint128 withdrawableAmount = _withdrawableAmountOf(streamId);
    if (withdrawableAmount > 0) {
        withdraw({ streamId: streamId, to: currentRecipient, amount: withdrawableAmount });
    }


    // Checks and Effects: transfer the NFT.
    _transfer({ from: currentRecipient, to: newRecipient, tokenId: streamId });
}`

`function _transfer(address from, address to, uint256 amount) internal virtual {
    _balances[from] = _balances[from] - amount;
    _balances[to] = _balances[to] + amount;
    emit Transfer(from, to, amount);
}`

As seen from above, internal function will transfer amount instead of streamID

Impact

Instead of transferring streamID, amount is transferred which is incorrect.

Tools Used

Manual Review

Recommendations

Transfer streamID instead of amount