AuditorPraise - Missing circuit breaker checks in optimismRewardCollector.getExpectedExchange() while querying prices

[github-actions]

AuditorPraise

medium

Missing circuit breaker checks in optimismRewardCollector.getExpectedExchange() while querying prices

Summary

see vulnerability detail.

Vulnerability Detail

optimismRewardCollector.getExpectedExchange() relies on chainlink oracle to get price

        (
            uint80 roundID,
            int256 opToUsd,
            ,
            uint256 updateTime,
            uint80 answeredInRound
        ) = IChainlinkOracle(opToUsdOracle).latestRoundData();
        
        require(
            opToUsd > 0, // @audit-issue min/max not 0
            "Chainlink Malfunction"
        );

       (
            uint80 roundIDEth,
            int256 ethToUsd,
            ,
            uint256 updateTimeEth,
            uint80 answeredInRoundEth
        ) = IChainlinkOracle(ethToUsdOracle).latestRoundData();
        
        require(
            ethToUsd > 0, // @audit-issue min/max not 0
            "Chainlink Malfunction"
        ); 

As seen from above, there is no check to ensure that returned answer does not go below or above a certain price.

Chainlink aggregators have a built in circuit breaker if the price of an asset goes outside of a predetermined price band. Therefore, if op & eth experiences a huge drop/rise in value, the op/Usd & eth/Usd price feed will continue to return minAnswer/maxAnswer instead of the actual price of op & eth.

This will be problematic because it could potentially lead to a scenario where the protocol interacts with the assets using incorrect price information.

Impact

Missing circuit breaker checks in optimismRewardCollector.getExpectedExchange() while querying prices allows protocol to use incorrect price information if op / eth experiences a huge drop/rise in value

Code Snippet

https://github.com/sherlock-audit/2024-04-alchemix/blob/main/v2-foundry/src/utils/collectors/OptimismRewardCollector.sol#L106

https://github.com/sherlock-audit/2024-04-alchemix/blob/main/v2-foundry/src/utils/collectors/OptimismRewardCollector.sol#L124

Tool used

Manual Review

Recommendation

Implement some sort of circuit breaker, so if the price goes below or above a threshold, it reverts the transaction; just as recommended by Chainlink here:: https://docs.chain.link/data-feeds/selecting-data-feeds#market-failures-resulting-from-extreme-events

[ABDuullahi]

Escalate

This should be a dupp of issue 80

[sherlock-admin3]

Escalate

This should be a dupp of issue 80

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[WangSecurity]

As it was correctly notified by Watsons on Discord, minAnswer and maxAnswer are no longer used on most of the Feeds, hence, this report should be invalid. If escalation on #14 is accepted and it's invalid, will reject the escalation here. If #14 is rejected, then this escalation will be accepted and de-duplicated.

[AuditorPraise]

As it was correctly notified by Watsons on Discord, minAnswer and maxAnswer are no longer used on most of the Feeds, hence, this report should be invalid. If escalation on #14 is accepted and it's invalid, will reject the escalation here. If #14 is rejected, then this escalation will be accepted and de-duplicated.

The escalation is around how this issue is missed as a dup of #80 when in fact they are clearly the same.

Either way i think since the escalation is solely on how the issue is missed out as a dup of #80 , it's rejection or acceptance should lie solely on that.

[WangSecurity]

@AuditorPraise thank you for that comment and remark. But under Sherlock's rules of escalations, if #14 will be invalid, then this escalation has to be rejected, since it doesn't effect the reward distribution

[Hash01011122]

Agreed with @WangSecurity, this can be invalidated

[Evert0x]

Result:
Invalid
Unique

[sherlock-admin4]

Escalations have been resolved successfully!

Escalation status:

• ABDUullahi: rejected