Sablier

Summary

The Transfer admin function is not safe.

Vulnerability Details

The transfer admin function doesn't check if the new admin address is correct nor if it is address(0) before applying the changes.

Impact

A simple mistake in the new admin address could lead to losing ownership.

Tools Used

Recommendations

Add a safe transfer admin by :

    - adding some checks : reject address(0)

    - adding a 'newAdmin' variable and a function to accept the new admin, in case the new admin address is wrong, just add a new one before accepting it

///////////////////////////////////////////////////////////////////////////
STATE VARIABLES
///////////////////////////////////////////////////////////////////////////

// @inheritdoc IAdminable
address public override admin;
// Audit: add a newAdmin variable
address public newAdmin;

///////////////////////////////////////////////////////////////////////////
USER-FACING NON-CONSTANT FUNCTIONS
///////////////////////////////////////////////////////////////////////////

function transferAdmin(address _newAdmin) public virtual override onlyAdmin {
// prevent from losing admin role forever | check for !address(0)
require(_newAdmin != address(0));
// add a temporary newAdmin address waiting for confirmation (only the new admin can accept the transfer, preventing wrong address)
newAdmin = _newAdmin;
}

function acceptNewAdmin() public {
    require(msg.sender == newAdmin);

    address oldAdmin = admin;
    admin = newAdmin;

    // clear newAdmin
    newAdmin = address(0);
    // Log the transfer of the admin.
    emit IAdminable.TransferAdmin({ oldAdmin: admin, newAdmin: newAdmin });
}