search

code-423n4 / 2022-04-mimo-findings

0 stars 0 forks source link

ChainlinkInceptionPriceFeed can report stale price #155

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/core/contracts/inception/priceFeed/ChainlinkInceptionPriceFeed.sol#L73-L80

Vulnerability details

As stale price is determined by time since last timestamp, the price that is most recent, but wasn't updated for more than _PRICE_ORACLE_STALE_THRESHOLD (say there were no trades on the market) will be rejected, which makes system unavailable in such a case. This can be deemed as a conservative choice.

In the same time real stale price will be accepted as long as it happens to be within _PRICE_ORACLE_STALE_THRESHOLD window. I.e. getAssetPrice do not require the price to be the most recent / fresh, simply accepting any price given that it is not older than _PRICE_ORACLE_STALE_THRESHOLD.

Proof of Concept

getAssetPrice ignores round data, only requiring the timestamp to be new enough:

https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/core/contracts/inception/priceFeed/ChainlinkInceptionPriceFeed.sol#L73-L80

  function getAssetPrice() public view override returns (uint256 price) {
    (, int256 eurAnswer, , uint256 eurUpdatedAt, ) = _eurOracle.latestRoundData();
    require(eurAnswer > 0, "EUR price data not valid");
    require(block.timestamp - eurUpdatedAt < _PRICE_ORACLE_STALE_THRESHOLD, "EUR price data is stale");

    (, int256 answer, , uint256 assetUpdatedAt, ) = _assetOracle.latestRoundData();
    require(answer > 0, "Price data not valid");
    require(block.timestamp - assetUpdatedAt < _PRICE_ORACLE_STALE_THRESHOLD, "Price data is stale");

I.e., recent timestamp doesn't guarantee fresh price, while fresh price can have non-recent timestamp (simply because of slow trading), so this check alone doesn't provide much protection.

Recommended Mitigation Steps

Consider imposing checks for positive price and for the round id, timestamp fields as it's suggested in the docs:

https://docs.chain.link/docs/feed-registry/#latestrounddata

(int256 roundID, int256 priceInUsd, , int256 updatedAt, int256 answeredInRound) = baseAggregator.latestRoundData();

require(priceInUsd > 0 && updatedAt > 0 && answeredInRound >= roundID , "Price invalid");
m19 commented 2 years ago

Duplicate of #1