sherlock-audit / 2023-04-unitasprotocol-judging

4 stars 3 forks source link

okolicodes - `Initializers` could be `frontrun`. #133

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

okolicodes

medium

Initializers could be frontrun.

Line with issue

Summary

The initializer in the Unitas.sol contracts could be frontrun by anyone.

Vulnerability Detail

As you can see the initialize funtion:

    function initialize(InitializeConfig calldata config_) public initializer {
        __Pausable_init();
        __AccessControl_init();
        __ReentrancyGuard_init();
        _setRoleAdmin(GOVERNOR_ROLE, GOVERNOR_ROLE);
        _setRoleAdmin(GUARDIAN_ROLE, GUARDIAN_ROLE);
        _setRoleAdmin(TIMELOCK_ROLE, GOVERNOR_ROLE);
        _setRoleAdmin(PORTFOLIO_ROLE, GUARDIAN_ROLE);

        _grantRole(GOVERNOR_ROLE, config_.governor);
        _grantRole(GUARDIAN_ROLE, config_.guardian);
        _grantRole(TIMELOCK_ROLE, config_.timelock);
        _grantRole(PORTFOLIO_ROLE, config_.guardian);

        _setOracle(config_.oracle);
        _setSurplusPool(config_.surplusPool);
        _setInsurancePool(config_.insurancePool);
        _setTokenManager(config_.tokenManager);
    }

Is called in after the contract is deployed allowing for a malicious actor to frontrun the transaction that would be use to initialize it.

Impact

Waste of time and also waste of gas for the deployers.

Code Snippet

Manual Review

Recommendation

Call the intialize function inside of the constructor along with the _disableInitialzers function after it:

    constructor(InitializeConfig calldata config_) {
     initialize( config_);
   _disableInitializers();
    }