Can initialize PRICE module with old market data

[code423n4]

Lines of code

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

Vulnerability details

Impact

The PRICE module's initialize function checks the lastObservationTime_ for dates in the future, but does not check if the last observation time is too old to be used as it does instead in the getCurrentPrice's for the price stream. The Deployer.sol script mentions "Actual market data will be used to initialize in production" therefore we can assume the same logic should apply. Initializing with old data would result in an invalid moving average output.

Proof of Concept

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

    // Check that the number of start observations matches the number expected
    if (startObservations_.length != numObs || lastObservationTime_ > uint48(block.timestamp))
        revert Price_InvalidParams();

Tools Used

none

Recommended Mitigation Steps

    // Check that the number of start observations matches the number expected
    // Use a multiple of observation frequency to determine what is too old to use
    if (startObservations_.length != numObs || lastObservationTime_ > uint48(block.timestamp))
            || lastObservationTime_ < uint48(block.timestamp - 1 * uint256(observationFrequency))
        revert Price_InvalidParams();

[Oighty]

While this is true, the same could be said for providing old data points and falsifying a last observation time. The primary purpose of last observation time is to provide an idea of when the moving average was last updated when it's running perpetually. Here we just provide an initialized value. The impact of this being wrong is really just incorrect data shown for a brief period of time until the price is updated for the first time.

[0xean]

downgrading to QA