sherlock-audit / 2024-05-midas-judging

13 stars 6 forks source link

vangrim - `_getDataInBase18()` could return the wrong price in case of a sudden price crash between EUR/USD pricing #53

Closed sherlock-admin4 closed 6 months ago

sherlock-admin4 commented 6 months ago

vangrim

medium

_getDataInBase18() could return the wrong price in case of a sudden price crash between EUR/USD pricing

Summary

The returned value from _getDatainBase18() might return a value that is lower or higher than the intended minAmountToDepositInUsd() due to the minAnswer and maxAnswer from the Chainlink aggregator contract leading to a green listed user being able to buy T-bills at a discounted/inflated price.

Vulnerability Detail

The best practice is to fetch the required data feed through the AggregatorV3Interface and run functions on the proxy contract instead. However, the actual Chainlink aggregator contract is called AccessControlledOffChainAggregator, which also contains circuit breakers that return the minAnswer and maxAnswer in case of sudden price fluctuations. This means a fixed value will be returned if the 'minAnswerormaxAnswer` has been reached, regardless of the actual value.

Impact

A green-listed user might be able to buy short-dated T-bills at a discounted price if the actual value of the function minAmountToDepositInUsd() is higher than the returned value.

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

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

Either override the circuit-breakers by the Chainlink aggregators to instead revert or add custom circuit-breakers that are more in line with Midas' intentions. Pseudo-code below for the _getDataInBase18() function:

...
        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"
        );
       require(_answer >= maxPrice, "DF: answer larger than maxPrice");
       require (_answer >= minPrice, "DF: answer smaller than minPrice");