Validity checks in `OracleHelper#_validateAnswer` are not sufficient and the function can return a stale price

[c4-bot-9]

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/main/contracts/WiseOracleHub/OracleHelper.sol#L155 https://github.com/code-423n4/2024-02-wise-lending/blob/main/contracts/WiseOracleHub/OracleHelper.sol#L168 https://github.com/code-423n4/2024-02-wise-lending/blob/main/contracts/WiseOracleHub/Declarations.sol#L109

Vulnerability details

The price retrieved from the Chainlink price feed in OracleHub undergoes two validity checks .

First One :

The price is compared against minAnswer and maxAnswer values extracted from the token aggregator

    function _compareMinMax(IAggregator _tokenAggregator, int192 _answer) internal view {
        int192 maxAnswer = _tokenAggregator.maxAnswer();
        int192 minAnswer = _tokenAggregator.minAnswer();

        if (_answer > maxAnswer || _answer < minAnswer) {
            revert OracleIsDead();
        }
    }

and if the price is under minAnswer or over maxAnswer the transactions revert preventing the operation to be executed with a stale price . However, we can read from chainlink docs that those values are no longer used in most data Feeds, and the team on chainlink discorde suggest refraining from using them any longer, so they are likely not to exposed in the next aggregators versions .

Second One :

Getting the difference between the Chainlink price and a Uniswap TWAP and if its above a certain value the transaction will revert protecting the protcol .

    function _compareDifference(uint256 _relativeDifference) internal view {
        if (_relativeDifference > ALLOWED_DIFFERENCE) {
            revert OraclesDeviate();
        }
    }

But when the Uniswap pool oracle is not added to this token, this makes the fetchTwapValue to be 0 and the difference check will be skipped .

Also, a high volatility market event of an asset is not considered in TWAP_PERIOD as it cannot be changed after deployment of the contract. In fact 30 minutes window can result in getting a stale TWAP in sudden movement of the asset market .

Proof of Concept

consider an asset with the following particularities :

1. its chainlink aggregator does not contain a maxAnswer so the first verification is bypassed .
2. Its Uniswap TWAP Oracle is not set for any particular reason so fetchTwapValue will be 0 and the second verification is bypassed .

OR

1. the market is in high activity, so the 30 minutes TWAP Window will be too high compared to the asset volatiliy thus the retrieved TWAP is stale. Despite its staleness, it may align enough closely with the Chainlink price making the second verification pass even if both of them are outdated.

Impact

Users can exploit this by borrowing against an inflated price asset .

Tool Used

Manual review

Recommended Mitigation Steps

• use the updatedAt value from the latestRoundData() function to make sure that the latest answer is recent enough for you to use it instead .
• TWAP_PERIOD value should be changeable (and not constant) by the admin/owner since it is dependent upon market conditions and activity .

Assessed type

Oracle

[GalloDaSballo]

OOS

[c4-pre-sort]

GalloDaSballo marked the issue as insufficient quality report

[c4-judge]

trust1995 marked the issue as unsatisfactory: Out of scope