Summary

In multiple instances, protocol indirectly uses create() to deploy a contract, since the new keyword is being used for the deployment, this method however leaves the approach susceptible to a re-org attack.

Vulnerability Details

Take a look at https://github.com/Cyfrin/2024-05-Sablier/blob/43d7e752a68bba2a1d73d3d6466c3059079ed0c6/v2-periphery/src/SablierV2MerkleLockupFactory.sol#L25-L41

This method uses the new keyword when deploying, which makes it susceptible to a re-org attack, which means an attacker can take control of the to-deploy contract while the chain is down/ or the re-org is going on. considering protocol is to be deployed on multiple L2s including optimistic ones then the chance of this occurring is quite high.

Would be key to note that the above is only one instance in scope, however other instances exist in scope where the new keyword is used for deployment and can be pinpointed by this search command: https://github.com/search?q=repo%3ACyfrin%2F2024-05-Sablier+%3D+new+NOT+language%3AMarkdown+NOT+language%3AShell&type=code, essentially how this bug case works is that the to-deploy address would be taken over by an attacker due to the re-org, considering assets are directly sent to the LockUp contracts by the sender for the later distribution to the receivers, this then means that this funds could be lost to the attacker who has overtaken the address.

Impact

Current deployment method of the LockUp contracts is unsafe considering a re-org attack would allow an attacker take over the contract, also protocol has stated they are to deploy on any EVM compatible chain which then heavily increases the likelihood of this occurring.

Tools Used

Manual review

Recommendations

Consider using create2 and a non-constant salt value when deploying.