sherlock-audit / 2024-04-teller-finance-judging

10 stars 9 forks source link

EgisSecurity - __Ownable_init is missing in LenderCommitmentGroup_Smart and TellerV2 #172

Closed sherlock-admin4 closed 4 months ago

sherlock-admin4 commented 4 months ago

EgisSecurity

high

__Ownable_init is missing in LenderCommitmentGroup_Smart and TellerV2

Summary

__Ownable_init is missing in LenderCommitmentGroup_Smart and TellerV2

Vulnerability Detail

The contracts inherits OwnableUpgradeable from OZ. In order to set the ownership when the contract is initialized, we have to call __Ownable_init, but it's missing from the initialize functions.

TellerV2.sol

function initialize(
        uint16 _protocolFee,
        address _marketRegistry,
        address _reputationManager,
        address _lenderCommitmentForwarder,
        address _collateralManager,
        address _lenderManager,
        address _escrowVault
    ) external initializer {
        __ProtocolFee_init(_protocolFee);

        __Pausable_init();

        require(
            _lenderCommitmentForwarder.isContract(),
            "LenderCommitmentForwarder must be a contract"
        );
        lenderCommitmentForwarder = _lenderCommitmentForwarder;

        require(
            _marketRegistry.isContract(),
            "MarketRegistry must be a contract"
        );
        marketRegistry = IMarketRegistry(_marketRegistry);

        require(
            _reputationManager.isContract(),
            "ReputationManager must be a contract"
        );
        reputationManager = IReputationManager(_reputationManager);

        require(
            _collateralManager.isContract(),
            "CollateralManager must be a contract"
        );
        collateralManager = ICollateralManager(_collateralManager);

        _setLenderManager(_lenderManager);
        _setEscrowVault(_escrowVault);
    }

LenderCommitmentGroup_Smart.sol

function initialize(
        address _principalTokenAddress,
        address _collateralTokenAddress,
        uint256 _marketId,
        uint32 _maxLoanDuration,
        uint16 _interestRateLowerBound,
        uint16 _interestRateUpperBound,
        uint16 _liquidityThresholdPercent, // When 100% , the entire pool can be drawn for lending.  When 80%, only 80% of the pool can be drawn for lending. 
        uint16 _collateralRatio, //the required overcollateralization ratio.  10000 is 1:1 baseline , typically this is above 10000
        uint24 _uniswapPoolFee,
        uint32 _twapInterval
    ) external initializer returns (address poolSharesToken_) {
        // require(!_initialized,"already initialized");
        // _initialized = true;

        __Pausable_init();

        principalToken = IERC20(_principalTokenAddress);
        collateralToken = IERC20(_collateralTokenAddress);

        UNISWAP_V3_POOL = IUniswapV3Factory(UNISWAP_V3_FACTORY).getPool(
            _principalTokenAddress,
            _collateralTokenAddress,
            _uniswapPoolFee
        );

        require(UNISWAP_V3_POOL != address(0), "Invalid uniswap pool address");

        marketId = _marketId;

        //in order for this to succeed, first, that SmartCommitmentForwarder needs to be THE trusted forwarder for the market

        ITellerV2Context(TELLER_V2).approveMarketForwarder(
            _marketId,
            SMART_COMMITMENT_FORWARDER
        );

        maxLoanDuration = _maxLoanDuration;
        interestRateLowerBound = _interestRateLowerBound;
        interestRateUpperBound = _interestRateUpperBound;

        require(interestRateLowerBound <= interestRateUpperBound, "invalid _interestRateLowerBound");

        require(_liquidityThresholdPercent <= 10000, "invalid _liquidityThresholdPercent"); 

        liquidityThresholdPercent = _liquidityThresholdPercent;
        collateralRatio = _collateralRatio;
        twapInterval = _twapInterval;

        poolSharesToken_ = _deployPoolSharesToken();
    }

Impact

pauseBorrowing and unpauseBorrowing won't work in LenderCommitmentGroup_Smart. pauseProtocol and unpauseProtocol won't work in TellerV2.

Code Snippet

https://github.com/sherlock-audit/2024-04-teller-finance/blob/defe55469a2576735af67483acf31d623e13592d/teller-protocol-v2-audit-2024/packages/contracts/contracts/TellerV2.sol#L188-L227 https://github.com/sherlock-audit/2024-04-teller-finance/blob/defe55469a2576735af67483acf31d623e13592d/teller-protocol-v2-audit-2024/packages/contracts/contracts/LenderCommitmentForwarder/extensions/LenderCommitmentGroup/LenderCommitmentGroup_Smart.sol#L158-L213

Tool used

Manual Review

Recommendation

Add __Ownable_init in initialize in both contracts

Duplicate of #35