yixxas - Chainlink oracle pricing will return a wrong price for asset if underlying aggregator hits minPrice

[sherlock-admin]

yixxas

medium

Chainlink oracle pricing will return a wrong price for asset if underlying aggregator hits minPrice

Summary

Chainlink oracles have a built in mechanism to prevent erroneous prices. If the price of underlying asset falls below minPrice, it will always return minPrice. In the event of an asset crash, this can be abused as the actual price of asset is considered to be higher by the oracle than what it should be.

Vulnerability Detail

Chainlink registry calls latestRoundData to get the answer at some roundId. If the actual price of the asset drops below minPrice, price will be set to minPrice. Leverages that can be made by users depend on their maintenance margin. Users can use this "overpriced asset" as collateral to create a maintenance margin higher than it should be, putting protocol into bad debt.

    function getLatestRound(ChainlinkRegistry self, address base, address quote) internal view returns (ChainlinkRound memory) {
        (uint80 roundId, int256 answer, , uint256 updatedAt, ) =
            FeedRegistryInterface(ChainlinkRegistry.unwrap(self)).latestRoundData(base, quote);
        return ChainlinkRound({roundId: roundId, timestamp: updatedAt, answer: answer});
    }

Impact

In the event of an asset crash, protocol can basically be made bankrupt by abusing an incorrectly priced asset.

Code Snippet

https://github.com/sherlock-audit/2023-05-perennial/blob/main/perennial-mono/packages/perennial-oracle/contracts/types/ChainlinkRegistry.sol#L34-L38

Tool used

Manual Review

Recommendation

Consider reverting if returned answer hits minPrice or maxPrice as it likely means that answer is no longer reliable.

[arjun-io]

These prices are not used to price the collateral, but instead used in deltas to determine PnL. If the answer goes below minAnswer and chainlink returns minAnswer each time, then the delta between the two prices will be 0 effectively bringing PnL to 0. Effectively the payoff function for these markets is max(price, minAnswer) which is still a valid payoff that doesn't negatively affect the Perennial market itself in terms of account/core flows.

[KenzoAgada]

@arjun-io is the maintenance not calculated via the Chainlink price? Am I missing something?

[arjun-io]

That's correct, but the market itself is essentially zero sum with respect to makers and takers net PnL - because they're both operating on the same payoff modification here there shouldn't be any adverse effects to the overall protocol - i.e accounting discrepancies leading to shortfall. Let us know if there's a deeper issue we're missing here, but from the protocol's view this is just a payoff function with a floor.

[KenzoAgada]

While the market is zero sum, I was thinking more from the viewpoint of specific users rather than the whole protocol, eg. users will not be exposed to the price deltas between minAnswer and 0.

But anyway, in addition to the sponsor's comments above, Looking at the actual minAnswer used by Chainlink for the ETH oracles, it is 1 wei and 1 gwei for Ethereum and Arbitrum. I've looked at a few other feeds and looks like the minAnswer is similarly tiny in most of them. Closing the issue as invalid or no impact.