Chainlink oracle upgrade may cause DOS for some tokens in the protocol

[c4-bot-1]

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseOracleHub/OracleHelper.sol#L19 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseOracleHub/OracleHelper.sol#L41

Vulnerability details

Impact

The pricefeed addresses and the aggregator addresses of tokens can be set by the master timelock through WiseOracleHub.addOracleBulk and WiseOracleHub.addAggregator which call internal functions in OracleHelper. These functions set the pricefeed and aggregator addresses after checking that the addresses have not previosly been set.

 function _addOracle(
        address _tokenAddress,
        IPriceFeed _priceFeedAddress,
        address[] calldata _underlyingFeedTokens
    )
        internal
    {
@>     if (priceFeed[_tokenAddress] > ZERO_FEED) {
            revert OracleAlreadySet();
        }

        priceFeed[_tokenAddress] = _priceFeedAddress;

        _tokenDecimals[_tokenAddress] = IERC20(
            _tokenAddress
        ).decimals();

        underlyingFeedTokens[_tokenAddress] = _underlyingFeedTokens;
    }

 function _addAggregator(
        address _tokenAddress
    )
        internal
    {
        IAggregator tokenAggregator = IAggregator(
            priceFeed[_tokenAddress].aggregator()
        );

@>      if (tokenAggregatorFromTokenAddress[_tokenAddress] > ZERO_AGGREGATOR) {
            revert AggregatorAlreadySet();
        }

       .................................................
    }

If the chainlink pricefeed or aggregator address for a token or token pair becomes stuck permanently in a stale state or if these addresses ever get upgraded by chainlink in the future, this would cause all oracle functions for that token or token pair to be permanently DOS as these pricefeed addresses cannot be overwritten.

Proof of Concept

This scenario can occur if the pricefeed or aggregator oracle of a token gets stuck in a stale state permanently or for a significant period of time, or if the address ever gets upgraded by chainlink.

Tools Used

Manual review

Recommended Mitigation Steps

1. Aggregator and pricefeed addresses for a given token or token pair should be changeable by a master preferrably a governance contract with a timelock. Consider removing the checks for if address is already set.

   function _addOracle(
       address _tokenAddress,
       IPriceFeed _priceFeedAddress,
       address[] calldata _underlyingFeedTokens
   )
       internal
   {
   
   -      if (priceFeed[_tokenAddress] > ZERO_FEED) {
   -          revert OracleAlreadySet();
       }
   
       priceFeed[_tokenAddress] = _priceFeedAddress;
   
       _tokenDecimals[_tokenAddress] = IERC20(
           _tokenAddress
       ).decimals();
   
       underlyingFeedTokens[_tokenAddress] = _underlyingFeedTokens;
   }

function _addAggregator(
        address _tokenAddress
    )
        internal
    {
        IAggregator tokenAggregator = IAggregator(
            priceFeed[_tokenAddress].aggregator()
        );

-      if (tokenAggregatorFromTokenAddress[_tokenAddress] > ZERO_AGGREGATOR) {
-            revert AggregatorAlreadySet();
        }

       if (_checkFunctionExistence(address(tokenAggregator)) == false) {
            revert FunctionDoesntExist();
        }

        UniTwapPoolInfo memory uniTwapPoolInfoStruct = uniTwapPoolInfo[
            _tokenAddress
        ];

        if (uniTwapPoolInfoStruct.oracle > ZERO_ADDRESS) {
            revert AggregatorNotNecessary();
        }

        tokenAggregatorFromTokenAddress[_tokenAddress] = tokenAggregator;
    }

Assessed type

Oracle

[c4-pre-sort]

GalloDaSballo marked the issue as duplicate of #30

[c4-pre-sort]

GalloDaSballo marked the issue as sufficient quality report

[c4-judge]

trust1995 changed the severity to QA (Quality Assurance)