sherlock-audit / 2023-04-blueberry-judging

8 stars 5 forks source link

cducrest-brainbot - ChainlinkAdapterOracle can still give stale data #43

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

cducrest-brainbot

medium

ChainlinkAdapterOracle can still give stale data

Summary

The implemented fix to issue https://github.com/sherlock-audit/2023-02-blueberry-judging/issues/94 is not sufficient. The ChainlinkAdapterOracle can still give stale data.

Vulnerability Detail

The fix is the following:

    function getPrice(address token_) external view override returns (uint256) {
        // remap token if possible
        address token = remappedTokens[token_];
        if (token == address(0)) token = token_;

        uint256 maxDelayTime = timeGaps[token];
        if (maxDelayTime == 0) revert Errors.NO_MAX_DELAY(token_);

        // Get token-USD price
        uint256 decimals = registry.decimals(token, USD);
        (, int256 answer, , uint256 updatedAt, ) = registry.latestRoundData(
            token,
            USD
        );
        if (updatedAt < block.timestamp - maxDelayTime)
            revert Errors.PRICE_OUTDATED(token_);
+       if (answer <= 0) revert Errors.PRICE_NEGATIVE(token_); 

        return
            (answer.toUint256() * Constants.PRICE_PRECISION) / 10 ** decimals;
    }

There is no check regarding answeredInRound and roundId, so the price data could be carried over.

Impact

Correct price is detrimental to the protocol as a whole. It is required to guarantee proper borrow limits, liquidation conditions, etc ...

An incorrect price may result in loss of funds for users e.g. when they are liquidated due to an incorrect price while they should not have.

Code Snippet

https://github.com/sherlock-audit/2023-04-blueberry/blob/96eb1829571dc46e1a387985bd56989702c5e1dc/blueberry-core/contracts/oracle/ChainlinkAdapterOracle.sol#L77-L97

Tool used

Manual Review

Reference

https://github.com/code-423n4/2022-04-backd-findings/issues/17 https://docs.chain.link/data-feeds/historical-data

Recommendation

Add require(answeredInRound >= roundID, "Stale price"); as an additional check:

    function getPrice(address token_) external view override returns (uint256) {
        // remap token if possible
        address token = remappedTokens[token_];
        if (token == address(0)) token = token_;

        uint256 maxDelayTime = timeGaps[token];
        if (maxDelayTime == 0) revert Errors.NO_MAX_DELAY(token_);

        // Get token-USD price
        uint256 decimals = registry.decimals(token, USD);
-        (, int256 answer, , uint256 updatedAt, ) = registry.latestRoundData(
+        (uint80 roundID, int256 answer, uint256 updatedAt, uint80 answeredInRound) = registry.latestRoundData(
            token,
            USD
        );
        if (updatedAt < block.timestamp - maxDelayTime)
            revert Errors.PRICE_OUTDATED(token_);
       if (answer <= 0) revert Errors.PRICE_NEGATIVE(token_); 
+      if(answeredInRound < roundID) revert Errors.PRICE_OUTDATED(token_);

        return
            (answer.toUint256() * Constants.PRICE_PRECISION) / 10 ** decimals;
    }

Duplicate of #118