Sablier

Summary

Claiming a Merkle Lockup sets the sender param to the admin of the deployed lockup. This makes him the only eligible caller of cancel and renounce due to the _isCallerStreamSender internal call.

If the admin of the lockup is modified, the new admin would be unable to call these 2 functions as they refer to stale data.

Vulnerability Details

Whenever a Merkle Lockup is created, it assigns in the constructor an initialAdmin. The admin's responsibilities in the beginning are the following:

1. Call clawback if necessary

2. Call setNFTDescriptor

3. Transfer the admin role

4. Account for being sender of merkle lockup streams

In the event of admin modification, tasks 1-3 are checked against onlyAdmin modifier and would be delegated to the new admin address. However, the initialAdmin would remain assigned as stream sender since it is checked against the admin address at the time of claiming the stream. The 4 main admin responsibilities would be split between 2 addresses, where neither of the 2 would be able to perform all 4 admin roles. Furthermore, the senderAmount during cancelation is paid out back to the initialAdmin and would have to get transferred from there too. One would assume that all initialAdmin functionalities should be available for the currentAdmin, especially if the initialAdmin address would be deprecated/dropped by the Merkle lockup creator. Assuming Sablier's intent to operate in a B2B environment, a company that pays out funds through merkle streams could modify the admin parameter more often, leading to unexpected behaviour.

An issue based on the same root-cause (stale admin parameter) is listed under the Known Issues paragraph, however it describes a completely different impact related to onLockupStreamWithdrawn() hook callback which has optional implementation and non-core functionality (as it does not handle funds, opposed to cancel). Therefore, I believe, due to the different impact, this should be considered as a separate issue.

Impact

Unexpected behaviour

Tools Used

Manual review

Recommendations

Modify _isCallerStreamSender to _isCallerStreamSenderOrCurrentAdmin and check whether the caller is the current admin too, if yes, send the senderAmount during cancelation to him.