Missing transfer admin in a 2-step process. Plus, no check for address(0). Can result in locked funds
Summary
Admin should be transfered in a 2 steps process. Likewise, a check to prevent transfering to address(0) is missing.
Vulnerability Details
Current admin can call transferAdmin() with a non-desired address or even address(0) as parameter.
https://github.com/Cyfrin/2024-05-Sablier/blob/43d7e752a68bba2a1d73d3d6466c3059079ed0c6/v2-core/src/abstracts/Adminable.sol#L34-L40
Impact
The modifier onlyAdmin is used in SablierV2Lockup.setNFTDescriptor() and SablierV2MerkleLockup.clawback(). Main impact is having clawback() blocked, which could result in locked funds in the contract, when the Airstream campaing has expired and not all claims were made.
Tools Used
Manual check
Recommendation
Consider implementing a two step process where the admin nominates an account and the nominated account needs to call an acceptOwnership() function for the transfer of admin to fully succeed. This ensures the nominated EOA account is a valid and active account.
Lead Judging Commences
Info/Gas/Invalid as per Docs
https://docs.codehawks.com/hawks-auditors/how-to-determine-a-finding-validity
Prize pool breakdown
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.