Sablier

Summary

Overflow possible because of 'unchecked {}' used to save gas.

Vulnerability Details

The comments in the code say "overflow is not an issue because the transaction will revert if there is an overflow", but it won't because _handleTransfer() is outside the for loop. _handleTransfer() function handle the transfer of the total amount of the batch (sum of all amounts from the batch).
If an attacker is able to cheat this amount to transfer with an overflow attack, he will be able to only transfer a small amount instead of the real amount which is larger. And nothing will revert.

The code as it is right now:

    uint256 i;
    uint256 transferAmount;
    for (i = 0; i < batchSize; ++i) {
        unchecked {
            transferAmount += batch[i].totalAmount;
        }
    }

    // Perform the ERC-20 transfer and approve {SablierV2LockupDynamic} to spend the amount of assets.
    _handleTransfer(address(lockupDynamic), asset, transferAmount);

   // Create a stream for each element in the parameter array.
    streamIds = new uint256[](batchSize);
    for (i = 0; i < batchSize; ++i) {
        // Create the stream.
        streamIds[i] = lockupDynamic.createWithDurations(
            LockupDynamic.CreateWithDurations({
                sender: batch[i].sender,
                recipient: batch[i].recipient,
                totalAmount: batch[i].totalAmount,
                asset: asset,
                cancelable: batch[i].cancelable,
                transferable: batch[i].transferable,
                segments: batch[i].segments,
                broker: batch[i].broker
            })
        );
    }

// ...

// @dev Helper function to transfer assets from the caller to the batchLockup contract and approve the Sablier
// contract.

function _handleTransfer(address sablierContract, IERC20 asset, uint256 amount) internal {
    // Transfer the assets to the batchLockup contract.
    asset.safeTransferFrom({ from: msg.sender, to: address(this), value: amount });

    // Approve the Sablier contract to spend funds.
    _approve(sablierContract, asset, amount);
}

Small POC for the overflow with 'unchecked {}' :

pragma solidity >=0.8.22;

contract Test {

    uint256[2] public streamIds;

    function testUnchecked() public returns(uint256){

        uint256[2] memory batch;
        batch[0] = type(uint256).max;
        batch[1] = 100;

        uint256 batchSize = batch.length;
        uint256 transferAmount;

        for (uint256 i = 0; i < batchSize; ++i) {
                unchecked {
                    transferAmount += batch[i];
                }
        }

//_handleTransfer(...); //transfer the sum of each element of batch = transferAmount


    for (uint256 i = 0; i < batchSize; ++i) {
        // Create the stream.
        //......
        streamIds[i] = batch[i];
    }

    return (transferAmount);
 }

}

testUnchecked() returns 99.

So _handleTransfer(...) will transfer only 99 even if streamIds[0] = type(uint256).max and streamIds[1] = 100.

=> TransferAmount can overflow.

Then, by puting a sum of large numbers equal to type(uint256).max plus a last small number (in this example 100) _handleTransfer() will only
transfer the last number -1 (here 100-1 = 99) even if each element of batch will be stored as large amounts.

=> The streams will be created with amounts that are not transfered to the contract.
Should remove "unchecked" to keep checking for overflow because the transfer is done with the sum of all amounts and not each one individually.

Impact

Wrong amount Transfered to the Smart Contract, small amount instead of the real amount (greater) BUT the contract thinks the real amount is Transfered. Each amount of the batch is stored but the Transfer of the corresponding funds (sum of the batch amounts) is lesser.

Recommendations

Do not use 'unchecked {}' to avoid an overflow.