sherlock-audit / 2024-06-mellow-judging

7 stars 3 forks source link

0xjarix - `ADMIN_ROLE`, `ADMIN_DELEGATE_ROLE`, and `OPERATOR` roles at risk #277

Closed sherlock-admin2 closed 3 months ago

sherlock-admin2 commented 4 months ago

0xjarix

High

ADMIN_ROLE, ADMIN_DELEGATE_ROLE, and OPERATOR roles at risk

Summary

ADMIN_ROLE, ADMIN_DELEGATE_ROLE, and OPERATOR roles at risk as initialize(...)can be front-run.

Vulnerability Detail

ADMIN_ROLE, ADMIN_DELEGATE_ROLE, and OPERATOR roles at risk as initialize(...)can be front-run since anyone can call initialize(...) because of a lack of _disableInitializers() in the constructor. initialize(...) cannot be called a 2nd time even though initializer() modifier is not used, and that's because of the configurator variable that is set when initialize(...) is first called by the attacker.

Impact

All functions that should be only callable by admin or operator can be called by the attacker, the ones calling _requireAdmin()and using the modifiers atLeastOperator() and onlyAdmin(). The attacker will be able to configure the vault to his liking, add and remove tokens and modules, process withdrawls when he wants and more. He's in complete control of the protocol

Code Snippet

https://github.com/sherlock-audit/2024-06-mellow/blob/main/mellow-lrt/src/security/Initializer.sol#L14

constructor()
        ERC20("Initializer", "init")
        DefaultAccessControl(address(this)) // @audit _disableInitializers() ownership at risk
    {}

function initialize(
        string memory name_,
        string memory symbol_,
        address admin_
    ) external {
        if (address(configurator) != address(0)) revert();
        configurator = new VaultConfigurator();

        // copy and paste from DefaultAccessControl constructor
        if (admin_ == address(0)) revert AddressZero();
        _grantRole(OPERATOR, admin_);
        _grantRole(ADMIN_ROLE, admin_);

        _setRoleAdmin(ADMIN_ROLE, ADMIN_ROLE);
        _setRoleAdmin(ADMIN_DELEGATE_ROLE, ADMIN_ROLE);
        _setRoleAdmin(OPERATOR, ADMIN_DELEGATE_ROLE);

        // ...
        }
    }

Tool used

Manual Review

Recommendation

Fix the bug by adding _disableInitializers() to the constructor.