sherlock-audit / 2023-04-jojo-judging

7 stars 4 forks source link

0xhacksmithh - Returned Price could be a stale price in `getMarkPrice()` & `getAssetPrice()` #427

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

0xhacksmithh

medium

Returned Price could be a stale price in getMarkPrice() & getAssetPrice()

Summary

No check on returned price could give a negetive value and No check for round completeness could lead to stale prices and wrong price return value, or outdated price. The functions rely on accurate price feed might not work as expected, sometimes can lead to fund loss.

Vulnerability Detail

The oracle wrapper getMarkPrice() and getAssetPrice() call out to an oracle with latestRoundData() to get the price of some token. Although the returned timestamp is checked, there is no check for round completeness and returned price.

According to Chainlink's documentation, this function does not error if no answer has been reached but returns 0 or outdated round data. The external Chainlink oracle, which provides index price information to the system, introduces risk inherent to any dependency on third-party data sources. For example, the oracle could fall behind or otherwise fail to be maintained, resulting in outdated data being fed to the index price calculations. Oracle reliance has historically resulted in crippled on-chain systems, and complications that lead to these outcomes can arise from things as simple as network congestion.

Impact

If there is a problem with chainlink starting a new round and finding consensus on the new value for the oracle (e.g. chainlink nodes abandon the oracle, chain congestion, vulnerability/attacks on the chainlink system) consumers of this contract may continue using outdated stale data (if oracles are unable to submit no new round is started).

This could lead to stale prices and wrong price return value, or outdated price.

Code Snippet

https://github.com/sherlock-audit/2023-04-jojo/blob/main/smart-contract-EVM/contracts/adaptor/chainlinkAdaptor.sol#L46-L47

https://github.com/sherlock-audit/2023-04-jojo/blob/main/JUSDV1/src/oracle/JOJOOracleAdaptor.sol#L28-L29

Tool used

Manual Review

Recommendation

Make Proper Validations,

        (uint80 roundID, rawPrice, , updatedAt, uint80 answeredInRound) = IChainlink(chainlink).latestRoundData(); 
        (uint80 roundID1, int256 USDCPrice,, uint256 USDCUpdatedAt, uint80 answeredInRound1) = IChainlink(USDCSource).latestRoundData(); 
        require(
            block.timestamp - updatedAt <= heartbeatInterval, 
            "ORACLE_HEARTBEAT_FAILED"
        );
        require(block.timestamp - USDCUpdatedAt <= heartbeatInterval, "USDC_ORACLE_HEARTBEAT_FAILED");
+     // Price check: Validate the price falls within an expected range
+      require(rawPrice > 0, "Invalid price");
+     // Price check: Validate the price falls within an expected range
+       require(USDCPrice > 0, "Invalid price");

+       require(answeredInRound >= roundID, "Stale price");
+       require(answeredInRound1 >= roundID1, "Stale price");

        uint256 tokenPrice = (SafeCast.toUint256(rawPrice) * 1e8) / SafeCast.toUint256(USDCPrice);
        return tokenPrice * 1e18 / decimalsCorrection;

Duplicate of #173