sherlock-audit / 2024-05-midas-judging

4 stars 2 forks source link

aman - The Integration of Price Feed is not correct and vulnerable to stale price #157

Closed sherlock-admin3 closed 1 month ago

sherlock-admin3 commented 1 month ago

aman

medium

The Integration of Price Feed is not correct and vulnerable to stale price

Summary

The Protocol assume that the IB01/USD price feed will not get updated at weekends due to which they have used 3 days timeout for stale price but in reality it is not, Due to this large timeout the Price feed is vulnerable to stale price conversion.

Vulnerability Detail

Quoting this from Readme:

Are the admins of the protocols your contracts integrate with (if any) TRUSTED or RESTRICTED? If these integrations are trusted, should auditors also assume they are always responsive, for example, are oracles trusted to provide non-stale information, or VRF providers to respond within a designated timeframe?

IB01/USD Price from Chainlink is RESTRICTED, as their documentation states that the price will only be updated during defined market hours (weekends and holidays excluded), so we assume the price is only stale if more than three days have passed.

  1. According to Chainlink docs the Oracle will update it price in following cases
Name Description
Deviation Threshold Chainlink nodes are monitoring data offchain. The deviation of the real-world data beyond a certain interval triggers all the nodes to update.
Heartbeat Threshold If the data values stay within the deviation parameters, it will only trigger an update every X minutes / hours.

Have look here

  1. The Heart Beat for IB01/USD Price Feed is not 3 days it is 86400s .You can check it from here. From above two point it is clear that the Price Feed will update after every 86400s , So The Protocol assumes that it will not update from Friday to Sunday which is wrong. The ChainLink recommendation is that the Price may not be correct during off time of a market but they will update it after every 86400s. the correct time to check for stale price is 86400s not 3 days.

For Proof that it also update the Price Feed during weekends. Pass 18446744073709551980 to getRoundData function of AggregatorV3Interface the time at which the Price Feed was update is 1716106835 in Unix time stamp and Sun May 19 2024 08:20:35 GMT+0000 in GMT time zone.

    uint256 private constant _HEALTHY_DIFF = 3 days;

Impact

This _getDataInBase18() will not revert if price is stale. which will result in wrong calculation for conversion.

Code Snippet

https://github.com/sherlock-audit/2024-05-midas/blob/main/midas-contracts/contracts/feeds/DataFeed.sol#L64-L80

Tool used

Manual Review

Recommendation

Change _HEALTHY_DIFF from 3 days to 86400s

diff --git a/midas-contracts/contracts/feeds/DataFeed.sol b/midas-contracts/contracts/feeds/DataFeed.sol
index 4282ecf..e8d27b6 100644
--- a/midas-contracts/contracts/feeds/DataFeed.sol
+++ b/midas-contracts/contracts/feeds/DataFeed.sol
@@ -24,7 +24,7 @@ contract DataFeed is WithMidasAccessControl, IDataFeed {
     /**
      * @dev healty difference between `block.timestamp` and `updatedAt` timestamps
      */
-    uint256 private constant _HEALTHY_DIFF = 3 days;
+    uint256 private constant _HEALTHY_DIFF = 86400;

     /**
      * @inheritdoc IDataFeed

Duplicate of #110

WangSecurity commented 3 weeks ago

This report will be invalidated based on the escalation on #110.