code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-drips/blob/9fd776b50f4be23ca038b1d0426e63a69c7a511d/src/Managed.sol#L84

Vulnerability details

Impact

Admin of contracts is controlled by Managed contract, and implements a transfer of admin privilege in a single step.

A malicious admin or an error in the new address when calling changeAdmin() can prevent all admin activities on all the contracts forever.

Proof of Concept

Transfer of admin privilege:

    /// @notice Changes the admin of the contract.
    /// Can only be called by the current admin.
    function changeAdmin(address newAdmin) public onlyAdmin {
        _changeAdmin(newAdmin);
    }

Tools Used

Manual analysis.

Recommended Mitigation Steps

Recommend the transfer of admin privileges to be a two step process, like it is implemented in other projects, like Compound Timelock for instance, where you have a setPendingAdmin() and a acceptAdmin() calls to start and finalize the process respectively.

c4-judge commented 1 year ago

GalloDaSballo marked the issue as unsatisfactory: Overinflated severity

GalloDaSballo commented 1 year ago

Non Critical for a long time

code-423n4 / 2023-01-drips-findings