0xStalin - ChainlinkOracle will return a wrong price for the asset in case of market turbulence

[sherlock-admin]

0xStalin

medium

ChainlinkOracle will return a wrong price for the asset in case of market turbulence

Summary

Chainlink aggregators have implemented an automatic fail backup mechanism in case the price of an asset experiences a huge drop in value. The price of the oracle will continue to return the minPrice of the real price of the asset.
In the event that any of the approved collaterals, or even USDC experiences market turbulences and its price drops hard, the users will be able to continue borrowing JUSD with assets priced wrongly. This is exactly what happened when LUNA crashed, affecting a couple of protocols.

Vulnerability Detail

• getAssetPrice():JOJOOracleAdaptor.sol pulls the prices of the collateral asset and USDC from ChainlinkAggregators

function getAssetPrice() external view override returns (uint256) {
    /*uint80 roundID*/
    (, int256 price,, uint256 updatedAt,) = IChainLinkAggregator(chainlink).latestRoundData();
    (, int256 USDCPrice,, uint256 USDCUpdatedAt,) = IChainLinkAggregator(USDCSource).latestRoundData();

    require(block.timestamp - updatedAt <= heartbeatInterval, "ORACLE_HEARTBEAT_FAILED");
    require(block.timestamp - USDCUpdatedAt <= heartbeatInterval, "USDC_ORACLE_HEARTBEAT_FAILED");
    uint256 tokenPrice = (SafeCast.toUint256(price) * 1e8) / SafeCast.toUint256(USDCPrice);
    return tokenPrice * JOJOConstant.ONE / decimalsCorrection;
}

ChainlinkFeedRegistry:latestRoundData() pulls the associated aggregator and requests round data from it. ChainlinkAggregators have minPrice and maxPrice circuit breakers built into them. If the price of the asset drops below the minPrice, the protocol will continue to value the token at minPrice instead of its real value. This will allow users to take out huge amounts of bad debt and potentially bankrupt the protocol, or at least harming tremendously the solvency of the protocol reserves.

Example:

• TokenA has a minPrice of $1. The price of TokenA drops to $0.10. The aggregator still returns $1 allowing the user to borrow against TokenA as if it is $1 which is 10x it's actual value.

As per Chainlink Risk Mitigation Guide Lines

• Design your systems and smart contracts to be resilient and mitigate risk to your protocol and your users. Ensure that your systems can tolerate known and unknown exceptions that might occur.
  • Some examples include but are not limited to volatile market conditions ...

Impact

In the event that an asset crashes, the protocol can be manipulated to give out bad loans at an inflated price

Code Snippet

https://github.com/sherlock-audit/2023-04-jojo/blob/main/JUSDV1/src/oracle/JOJOOracleAdaptor.sol#L26-L35
https://github.com/sherlock-audit/2023-04-jojo/blob/main/smart-contract-EVM/contracts/adaptor/chainlinkAdaptor.sol#L43-L55

Tool used

Manual Review

Recommendation

• As per Chainlink Documentation, there are two variables that bound the highest and lowest acceptable prices that aggregators will accept.
• getAssetPrice() should pull the the aggregator's minPrice/maxPrice and use them to check and sanitize the returned price, and revert if the returned price is outside of the bounds

function getAssetPrice() external view override returns (uint256) {
        /*uint80 roundID*/
        (, int256 price,, uint256 updatedAt,) = IChainLinkAggregator(chainlink).latestRoundData();
        //fetch the pricefeeds hard limits
+      uint256 collateralAssetMinPrice =  IChainLinkAggregator(chainlink).minAnswer();
+      uint256 collateralAssetMaxPrice =  IChainLinkAggregator(chainlink).maxAnswer(); 
+      require(price < collateralAssetMinPrice , "Collateral Upper price bound breached");
+      require(price > collateralAssetMaxPrice , "Collateral Lower price bound breached");
   
        (, int256 USDCPrice,, uint256 USDCUpdatedAt,) = IChainLinkAggregator(USDCSource).latestRoundData();
        //fetch the pricefeeds hard limits
+     uint256 USDCMinPrice = IChainLinkAggregator(USDCSource).minAnswer();
+     uint256 USDCMaxPrice =  IChainLinkAggregator(USDCSource).maxAnswer(); 
+     require(USDCPrice < USDCMinPrice , "USDC Upper price bound breached");
+     require(USDCPrice > USDCMaxPrice , "USDC Lower price bound breached");

        require(block.timestamp - updatedAt <= heartbeatInterval, "ORACLE_HEARTBEAT_FAILED");
        require(block.timestamp - USDCUpdatedAt <= heartbeatInterval, "USDC_ORACLE_HEARTBEAT_FAILED");

        uint256 tokenPrice = (SafeCast.toUint256(price) * 1e8) / SafeCast.toUint256(USDCPrice);
        return tokenPrice * JOJOConstant.ONE / decimalsCorrection;
    }

• Consider implementing the same recomendation on the getMarkPrice():chainlinkAdaptor.sol

• Additionally, consider checking if the sequencer is up on the L2. as per Chainlink Documentation, consider adding the below code snippet on the two Chainlink Oracles to validate Sequencer is up and prevent users from abusing in case the sequencer is not running and a bad price is being returned

// Price == 0: Sequencer is up
// Price == 1: Sequencer is down
bool isSequencerUp = price == 0;
if (!isSequencerUp) {
    revert SequencerDown();
}

Duplicate of #101