sherlock-audit / 2023-03-taurus-judging

4 stars 0 forks source link

saian - Inherited contracts contains initializer instead of onlyInitializing #199

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

saian

medium

Inherited contracts contains initializer instead of onlyInitializing

Summary

Inherited contracts contains initializer modifier instead of onlyInitializing modifier

Vulnerability Detail

Contracts Basevault, tauDripFeed, ControllableUpgradeable are inherited in other contract. So the initialization functions should use onlyInializing modifier instead of initilizer. Nesting initializer modifier will result in revert during initialization.

Impact

Nested initializer will revert during initializing

Code Snippet

    modifier initializer() {
        bool isTopLevelCall = !_initializing;
        require(
            (isTopLevelCall && _initialized < 1) || (!AddressUpgradeable.isContract(address(this)) && _initialized == 1),
            "Initializable: contract is already initialized"
        );
        _initialized = 1;
        if (isTopLevelCall) {
            _initializing = true;
        }
        _;
        if (isTopLevelCall) {
            _initializing = false;
            emit Initialized(1);
        }
    }

https://github.com/sherlock-audit/2023-03-taurus/blob/main/taurus-contracts/contracts/Vault/BaseVault.sol#L21

    function __BaseVault_init(address _controller, address _tau, address _collateralToken) internal initializer {   
        __Controllable_init(_controller);
        __TauDripFeed_init(_tau, _collateralToken);
    }

https://github.com/sherlock-audit/2023-03-taurus/blob/main/taurus-contracts/contracts/Vault/TauDripFeed.sol#L11

    function __TauDripFeed_init(address _tau, address _collateralToken) internal initializer {  
        __Pausable_init();
        tau = _tau;
        collateralToken = _collateralToken;
    }

https://github.com/sherlock-audit/2023-03-taurus/blob/main/taurus-contracts/contracts/Controller/ControllableUpgradeable.sol#L16

    function __Controllable_init(address _controller) internal initializer {    
        controller = _controller;
    }

Tool used

Manual Review

Recommendation

Change initializer to onlyInitializing in inherited contracts

Duplicate of #34