search

sherlock-audit / 2024-05-midas-judging

13 stars 6 forks source link

y0ng0p3 - Chainlink oracle will return a wrong price if the aggregator hits minAnswer #94

Closed sherlock-admin3 closed 6 months ago

sherlock-admin3 commented 6 months ago

y0ng0p3

medium

Chainlink oracle will return a wrong price if the aggregator hits minAnswer

Summary

Chainlink aggregators have a built-in circuit breaker if the price of an asset goes outside of a predetermined price band.

The result is that if an asset experiences a huge drop in value (i.e. LUNA crash) the price of the oracle will continue to return the minPrice instead of the actual price of the asset and vice versa.

Vulnerability Detail

The DataFeed::_getDataInBase18 function is only checking for the stale price. But no checks are done to handle that.

    function _getDataInBase18()
        private
        view
        returns (uint80 roundId, uint256 answer)
    {
        uint8 decimals = aggregator.decimals();
        (uint80 _roundId, int256 _answer, , uint256 updatedAt, ) = aggregator
            .latestRoundData();
        require(_answer > 0, "DF: feed is deprecated");
        require(
            // solhint-disable-next-line not-rely-on-time
            block.timestamp - updatedAt <= _HEALTHY_DIFF,
            "DF: feed is unhealthy"
        );
        roundId = _roundId;
        answer = uint256(_answer).convertToBase18(decimals);
    }

[64-80]

There is no function for checking only this as well in the protocol. And this function is used in the contract when converting the minAmountToDepositInEuro to USD in base18.

    function minAmountToDepositInUsd() public view returns (uint256) {
        return
            (minAmountToDepositInEuro * eurUsdDataFeed.getDataInBase18()) /
            10**18;
    }

[137*141]

function getDataInBase18() external view returns (uint256 answer) {
        (, answer) = _getDataInBase18();
    }

[54-56]

Impact

This would allow new users to make their first deposits, but at the wrong price. This is exactly what happened to Venus on BSC when LUNA crashed.

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

Consider using the following changes:

    function _getDataInBase18()
        private
        view
        returns (uint80 roundId, uint256 answer)
    {
        uint8 decimals = aggregator.decimals();
        (uint80 _roundId, int256 _answer, , uint256 updatedAt, ) = aggregator
            .latestRoundData();
--      require(_answer > 0, "DF: feed is deprecated");
++      require(_answer > minPrice, "DF: feed is deprecated");
++      require(_answer < maxPrice, "DF: feed is deprecated");
        require(
            // solhint-disable-next-line not-rely-on-time
            block.timestamp - updatedAt <= _HEALTHY_DIFF,
            "DF: feed is unhealthy"
        );
        roundId = _roundId;
        answer = uint256(_answer).convertToBase18(decimals);
    }