L1:There's no check for startTime being in the future, which would underflow elapsedTime below

Summary

There's no check for _streams[streamId].startTime > block.timestamp. Resulting in a waste of gas to arrive to return 0.

Vulnerability Details

Contrasting with SablierV2LockupTranched._calculateStreamedAmount() and SablierV2LockupDynamic_calculateStreamedAmount(), there's no check for _streams[streamId].startTime > block.timestamp in SablierV2LockupLinear.

https://github.com/Cyfrin/2024-05-Sablier/blob/43d7e752a68bba2a1d73d3d6466c3059079ed0c6/v2-core/src/SablierV2LockupLinear.sol#L189-L208

Impact

When cliffTime is set to 0, there will be a waste of gas in unnecesary math. Further impact is prevented by the implementation of the bug check at the end of the function:

https://github.com/Cyfrin/2024-05-Sablier/blob/43d7e752a68bba2a1d73d3d6466c3059079ed0c6/v2-core/src/SablierV2LockupLinear.sol#L220-L225

Thus,return 0 will still be the outcome. However, there's no need to run all the above math wasting gas unnecesarily.

Tools Used

Manual check

Recommended Mitigation

Replace L193-L195 with the below code and remove L207.

L2: Some ERC20 tokens will revert on max approve

Summary

Some ERC20 tokens will revert on max approve.

Vulnerability Details

Affects
https://github.com/Cyfrin/2024-05-Sablier/blob/43d7e752a68bba2a1d73d3d6466c3059079ed0c6/v2-periphery/src/SablierV2MerkleLL.sol#L40-L52
And
https://github.com/Cyfrin/2024-05-Sablier/blob/43d7e752a68bba2a1d73d3d6466c3059079ed0c6/v2-periphery/src/SablierV2MerkleLT.sol#L40-L58

Not every ERC20 token allows for type(uint256).max approve. This is not solved by forceApprove() since its intention is different, as it can be seen in its natSpec:

Impact

Contract creation will revert in
https://github.com/Cyfrin/2024-05-Sablier/blob/43d7e752a68bba2a1d73d3d6466c3059079ed0c6/v2-periphery/src/SablierV2MerkleLockupFactory.sol#L25
And
https://github.com/Cyfrin/2024-05-Sablier/blob/43d7e752a68bba2a1d73d3d6466c3059079ed0c6/v2-periphery/src/SablierV2MerkleLockupFactory.sol#L43

Tools Used

Manual check

Recommendation

Use aggregateAmount instead of type(uint256).max

L3: ERC20 is not transfered to the MerkleLockup contract after creation.

Summary

ERC20 is approved in SablierV2MerkleLL.constructor() and SablierV2MerkleLT.constructor() but not transfered by sender. Should be transfered in SablierV2MerkleLockupFactory.createMerkleLL() and SablierV2MerkleLockupFactory.createMerkleLT(), following how the protocol handles it. To prevent claims to revert because of sender forgetting to do it afterwards.

Vulnerability Details

In the constructors of SablierV2MerkleLL and SablierV2MerkleLT , the ERC20 holded by the contracts is approved to be handled by SablierV2MerkleLockupFactory. But following protocol money workflow, a recipient can claim without having the ERC20 yet deposited. This would result in an unclear revert() as a result of a failed SablierV2LockupLinear.createWithDurations() or SablierV2LockupTranched.createWithDurations()

Example for Linear Airstream:
Call to
https://github.com/Cyfrin/2024-05-Sablier/blob/43d7e752a68bba2a1d73d3d6466c3059079ed0c6/v2-periphery/src/SablierV2MerkleLockupFactory.sol#L25-L40
Which triggers the constructor()
https://github.com/Cyfrin/2024-05-Sablier/blob/43d7e752a68bba2a1d73d3d6466c3059079ed0c6/v2-periphery/src/SablierV2MerkleLL.sol#L40-L52
When claiming
https://github.com/Cyfrin/2024-05-Sablier/blob/43d7e752a68bba2a1d73d3d6466c3059079ed0c6/v2-periphery/src/SablierV2MerkleLL.sol#L59-L95
The below line reverts
https://github.com/Cyfrin/2024-05-Sablier/blob/43d7e752a68bba2a1d73d3d6466c3059079ed0c6/v2-core/src/SablierV2LockupLinear.sol#L277

Impact

If the sender forgets to send funds before recipients claim, they won't be able to claim the stream.

Tools Used

Manual check

Recommendation

Do the transfer of ERC20 to the SablierV2MerkleLL and SablierV2MerkleLT contracts in SablierV2MerkleLockupFactory.SablierV2MerkleLockupFactory:L37 and SablierV2MerkleLockupFactory.SablierV2MerkleLockupFactory:L72