`WiseOracleHub.getTokensPriceInUSD` function uses the returned ETH price without validation

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0xd932d4999df7e0fe1e50b62447d2343df0fd6d557c2c4396bd4819c1d173a26a Severity: medium

Description:

Description

• WiseOracleHub.getTokensPriceInUSD function is designed to return the USD value of a token:

    function getTokensPriceInUSD(
        address _tokenAddress,
        uint256 _tokenAmount
    )
        external
        view
        returns (uint256)
    {
        return getTokensInETH(
            _tokenAddress,
            _tokenAmount
        )
            * getETHPriceInUSD()
            / 10 ** _decimalsUSD;
    }

  • first, it fetches the token/ETH value from a chainlink pricefeed,
  • then it extracts the ETH USD price via getETHPriceInUSD,
  • the token USD price will be the multiplication of the above fetched values.

• getETHPriceInUSD uses Chainlink.latestRoundData() to fetch ETH price data, where this call will return:

function latestRoundData() external view
    returns (
        uint80 roundId,
        int256 answer,
        uint256 startedAt,
        uint256 updatedAt,
        uint80 answeredInRound
    )

• where:

  • roundId: The round ID.
  • answer: The data that this specific feed provides. Depending on the feed you selected, this answer provides asset prices, reserves, NFT floor prices, and other types of data.
  • startedAt: Timestamp of when the round started.
  • updatedAt: Timestamp of when the round was updated.
  • answeredInRound: Deprecated - Previously used when answers could take multiple rounds to be computed.

• These returned values must be checked to verify that the returned price is fresh and not stale as this call might return stale/outdated asset price; such as checking that the returned updatedAt doesn't exceed a maximum delay specified by the protocol.

• But it was noticed that the price of ETH returned from getETHPriceInUSD is not checked or validated anywhere, and directly used to extract the token USD price:

  function getETHPriceInUSD()
        public
        view
        returns (uint256)
    {
        (
            ,
            int256 answer,
            ,
            ,
        ) = ETH_PRICE_FEED.latestRoundData();
  
        return uint256(
            answer
        );
    }

• Similar issue in WiseOracleHub.getTokensPriceFromUSD, where ut uses the returned price from getETHPriceInUSD() directly without validation.

Impact

Not checking the validity of the returned ETH price will result in using an invalid price and bricking the accounting in the protocol by returning an invalid token USD price.

Code Instance

WiseOracleHub.getTokensPriceInUSD function

    function getTokensPriceInUSD(
        address _tokenAddress,
        uint256 _tokenAmount
    )
        external
        view
        returns (uint256)
    {
        return getTokensInETH(
            _tokenAddress,
            _tokenAmount
        )
            * getETHPriceInUSD()
            / 10 ** _decimalsUSD;
    }

OracleHelper.getETHPriceInUSD function

function getETHPriceInUSD()
        public
        view
        returns (uint256)
    {
        (
            ,
            int256 answer,
            ,
            ,
        ) = ETH_PRICE_FEED.latestRoundData();

        return uint256(
            answer
        );
    }

Tool used

Manual Review

Recommendation

Update OracleHelper.getETHPriceInUSD function to validate the returned price feed data in a similar way used in OracleHelper._chainLinkIsDead (checking the sequence if in Arbitrum, the freshness, staleness and deviation of the returned price).

[DevHals]

Hi, this issue is different from the previously reported ones (by me), as the returned price from getETHPriceInUSD doesn't undergo the same validation checks used in the latestResolver() function ,

[vm06007]

Hello @DevHals, function getTokensPriceInUSD is not used to price any tokens or pools, or collateral or borrowed amount, All pools and correlated data is priced in ETH. We use only getTokensPriceInUSD for incentive pricing which does not require a check since it is for internal team allocation how much to allocate. You can also see that getTokensInETH() is capable of validating that converting USD to ETH feed is alive with which only matters in that case. It has hearbeat and TWAP protection when calling getTokensInETH()

[vm06007]

This is similar to: https://github.com/hats-finance/Wise-Lending-0xa2ca45d6e249641e595d50d1d9c69c9e3cd22573/issues/47 but both ruled out invalid since both do not understand that everything pool/collateral/borrow in protocol is nominated in ETH not in USD

[DevHals]

Hi @vm06007, I have noticed that this issue got invalidated, while issue #47 that is a duplicate of this issue got validated, please notice that this one was reported first,

[vm06007]

I've marked those with low that are still in question for now, I do agree that @Tri-pathi duplicated, but he would not agree with you that it is duplicated and would argue it is different. However, we are considering rewarding both should there be a consideration to change the code base for this

[vm06007]

will put low for now and check with @vonMangoldt about rewards going for @DevHals and @Tri-pathi for discussion about #47 (including blacklist discussion, for now inconclusive but based on latest internal talks with @vonMangoldt it will be desired functionality when it comes to blacklist) but for this issue I would refer to final comments on #47