code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-08-olympus/blob/b5e139d732eb4c07102f149fb9426d356af617aa/src/modules/PRICE.sol#L161-L173

Vulnerability details

Impact

Inconsistency in determining the getCurrentPrice for ohmEthPrice and reserveEthPrice.

Proof of Concept

        (, int256 ohmEthPriceInt, , uint256 updatedAt, ) = _ohmEthPriceFeed.latestRoundData();
        // Use a multiple of observation frequency to determine what is too old to use.
        // Price feeds will not provide an updated answer if the data doesn't change much.
        // This would be similar to if the feed just stopped updating; therefore, we need a cutoff.
        if (updatedAt < block.timestamp - 3 * uint256(observationFrequency))
            revert Price_BadFeed(address(_ohmEthPriceFeed));
        ohmEthPrice = uint256(ohmEthPriceInt);

        int256 reserveEthPriceInt;
        (, reserveEthPriceInt, , updatedAt, ) = _reserveEthPriceFeed.latestRoundData();
        if (updatedAt < block.timestamp - uint256(observationFrequency))
            revert Price_BadFeed(address(_reserveEthPriceFeed));
        reserveEthPrice = uint256(reserveEthPriceInt);

In getCurrentPrice(), the ohmEthPrice is validated at multiple of observationFrequency

if (updatedAt < block.timestamp - 3 * uint256(observationFrequency)) revert Price_BadFeed(address(_ohmEthPriceFeed));

But for reserveEthPriceInt , it is not as same above. if (updatedAt < block.timestamp - uint256(observationFrequency)) revert Price_BadFeed(address(_reserveEthPriceFeed));

Tools Used

Manual code review, VS code

Recommended Mitigation Steps

Use multiple of frequency for reserveEthPriceInt also, if (updatedAt < block.timestamp - 3 * uint256(observationFrequency)) revert Price_BadFeed(address(_reserveEthPriceFeed));

Oighty commented 2 years ago

Agree that these should be consistent and set based on the update parameters of the feeds, but don't believe this is a high severity issue.

Oighty commented 2 years ago

Duplicate of #441 .

0xean commented 2 years ago

downgrading to M per #441

code-423n4 / 2022-08-olympus-findings