Summary

Protocol indicated that one of the reasons of this audit is to ensure the protocol rightly works on other to-deploy chains.

Now protocol plans to deploy on optimistic L2 chains which include Optimism, Arbitrum, Blast... etc.

One thing with optimistic L2 chains is that they have a sequencer and whenever a protocol is to have any logic depending on time in their protocol the need to check if the sequencer went down during that time frame and if yes, integrate a logic that takes into account for how long the sequencer is down.

This is important because, in the case that an L2's sequencer is down, it's only going to be accepting forced transactions through the L1 inbox (or has just come back online), the state of the chain would be unpredictable and prone to manipulation, and why it's advised to not attempt trnasactions while the sequencer is down and actually have a grace period around when the sequencer comes back up so as to ensure everything has been settled... more logic on this idea can be seen from how AAVE treats their dependency on the sequencer, using their whitepaper we can see that there is a special provision the Price Oracle Sentinel does which is to ensure that a grace period is provided whenever the sequencer does go down.

Unfortunately no such logic is applied in Sablier's case, and when protocol does deploy on such chains, it leaves the timing logic to be potentially flawed.

Vulnerability Details

As heavily explained under Summary, this bug case is applicable to any logic depending on time or duration, but we are going to showcase a short example below:

The _hasGracePeriodPassed() method is directly used when attempting to clawback() assets here is it's implementation: https://github.com/Cyfrin/2024-05-Sablier/blob/43d7e752a68bba2a1d73d3d6466c3059079ed0c6/v2-periphery/src/abstracts/SablierV2MerkleLockup.sol#L154-L158

As previously hinted, if the sequencer is down, whenever it comes back up all price/timestamp updates are immediately processed, for example consider a case where the sequencer was down close to the time the grace period for clawing back assets would soon expire this then means that the admin would be unable to clawback these assets since the function above is directly being queried when attempting to claw back assets: https://github.com/Cyfrin/2024-05-Sablier/blob/43d7e752a68bba2a1d73d3d6466c3059079ed0c6/v2-periphery/src/abstracts/SablierV2MerkleLockup.sol#L107-L123

Impact

Since there is no sequencer uptime validation, logics dependent on timing/duration on optimistic L2 chains could easily broken.

Tools Used

Manual review

Recommendations

Consider integrating a logic that keeps in track of if the sequencer has gone down and for how long and in a case where this is the case, apply the duration of the downtime of the sequencer to the timing logics on the chain.