hash - `ChainlinkOracle` doesn't validate for minAnswer/maxAnswer

[sherlock-admin2]

hash

Medium

ChainlinkOracle doesn't validate for minAnswer/maxAnswer

Summary

ChainlinkOracle doesn't validate for minAnswer/maxAnswer

Vulnerability Detail

Current implementation of ChainlinkOracle doesn't validate for the minAnswer/maxAnswer values link

    function _getPriceWithSanityChecks(address asset) private view returns (uint256) {
        address feed = priceFeedFor[asset];
        (, int256 price,, uint256 updatedAt,) = IAggegregatorV3(feed).latestRoundData();
        if (price <= 0) revert ChainlinkUsdOracle_NonPositivePrice(asset);
        if (updatedAt < block.timestamp - stalePriceThresholdFor[feed]) revert ChainlinkUsdOracle_StalePrice(asset);
        return uint256(price);
    }

Chainlink still has feeds that uses the min/maxAnswer to limit the range of values and hence in case of a price crash, incorrect price will be used to value the assets allowing user's to exploit this incorrectness by depositing the overvalued asset and borrowing against it. Since the project plans to deploy in Any EVM-compatbile network, I am attaching the link to BNB/USD oracle which still uses min/maxAnswer and is one of the highest tvl tokens in BSC https://bscscan.com/address/0x137924d7c36816e0dcaf016eb617cc2c92c05782#readContract, similar check exists for ETH/USD

Impact

In the even of a flash crash, user's lenders will loose their assets

Code Snippet

https://github.com/sherlock-audit/2024-08-sentiment-v2/blob/0b472f4bffdb2c7432a5d21f1636139cc01561a5/protocol-v2/src/oracle/ChainlinkUsdOracle.sol#L114-L120

Tool used

Manual Review

Recommendation

If the price is outside the minPrice/maxPrice of the oracle, activate a breaker to reduce further losses

[z3s]

mixAnswer/ maxAnswer is optional and not on all feeds, so protocol don't impose the check anywhere.

[0xMR0]

Escalate

[sherlock-admin3]

Escalate

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[0xMR0]

This is wrongly excluded and should be considered valid as per sherlock rule.

Chainlink Price Checks: Issues related to minAnswer and maxAnswer checks on Chainlink's Price Feeds are considered medium only if the Watson explicitly mentions the price feeds (e.g. USDC/ETH) that require this check.

duplicate following above sherlock rule should also be considered valid.

[Emanueldlvg]

Hi @z3s ,thanks for fast judging. I agree with @0xMR0 , based on README contest, token scope will be :

If you are integrating tokens, are you allowing only whitelisted tokens to work with the codebase or any complying with the standard? Are they assumed to have certain properties, e.g. be non-reentrant? Are there any types of [weird tokens](https://github.com/d-xo/weird-erc20) you want to integrate?

Tokens are whitelisted, only tokens with valid oracles can be used to create Base Pools.

Protocol governance will ensure that oracles are only set for standard ERC-20 tokens (plus USDC/USDT)

and sherlock rule : Chainlink Price Checks: Issues related to minAnswer and maxAnswer checks on Chainlink's Price Feeds are considered medium only if the Watson explicitly mentions the price feeds (e.g. USDC/ETH) that require this check

[0xspearmint1]

escalate

The valid duplicates of this issue are #27 and #120

Sherlock rules have clearly stated the following:

Chainlink Price Checks: Issues related to minAnswer and maxAnswer checks on Chainlink's Price Feeds are considered medium only if the Watson explicitly mentions the price feeds (e.g. USDC/ETH) that require this check

#357 is invalid because they linked aggregators that are deprecated, here are the links to the current aggregators for USDC/USD, ETH/USD and BTC/USD, ALL of them have a min price of 1 since they are deprecated

#259 is invalid because the watson mentioned a priceFeed that has a minPrice = 1, this is a price feed that used to have a real minAnswer but chainlink deprecated it.

#337 is invalid for the same reasons as [#259]

[sherlock-admin3]

escalate

The valid duplicates of this issue are #27 and #120

Sherlock rules have clearly stated the following:

Chainlink Price Checks: Issues related to minAnswer and maxAnswer checks on Chainlink's Price Feeds are considered medium only if the Watson explicitly mentions the price feeds (e.g. USDC/ETH) that require this check

#357 is invalid because they linked aggregators that are deprecated, here are the links to the current aggregators for USDC/USD, ETH/USD and BTC/USD, ALL of them have a min price of 1 since they are deprecated

#259 is invalid because the watson mentioned a priceFeed that has a minPrice = 1, this is a price feed that used to have a real minAnswer but chainlink deprecated it.

#337 is invalid for the same reasons as [#259]

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[0xRizwan]

Chainlink Price Checks: Issues related to minAnswer and maxAnswer checks on Chainlink's Price Feeds are considered medium only if the Watson explicitly mentions the price feeds (e.g. USDC/ETH) that require this check

Sherlock rule clearly states to explicitly mentions the price feeds (e.g. USDC/ETH) and linking the price feeds address is additional information by auditors. I believe, all such issues following above rule should be Medium severity.

[Blngogo]

Thanks to spearmint for trying to invalidate most of the reports. I guess trying to help a buddy to get higher payout. However #357 describes the same root cause and impact as the duplicates, the links for the aggregators are not deprecated, if you check the tx history you can see there are active tx's going on, more specifically the USDC/USD link. Requirements per Sherlock are: "Chainlink Price Checks: Issues related to minAnswer and maxAnswer checks on Chainlink's Price Feeds are considered medium only if the Watson explicitly mentions the price feeds (e.g. USDC/ETH) that require this check". And this requirement is fulfilled in this report.

[cvetanovv]

For my decision on this escalation, I will entirely rely on the Sherlock rule:

Chainlink Price Checks: Issues related to minAnswer and maxAnswer checks on Chainlink's Price Feeds are considered medium only if the Watson explicitly mentions the price feeds (e.g. USDC/ETH) that require this check.

All the duplicate issues have mentioned the price feeds. We have no information that it is mandatory to provide links.

Planning to accept @0xMR0 escalation and make this issue Medium severity, including all duplicates.

[Evert0x]

Result: Medium Has Duplicates

[sherlock-admin4]

Escalations have been resolved successfully!

Escalation status:

• 0xMR0: accepted
• 0xspearmint1: rejected