code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-07-moonwell/blob/fced18035107a345c31c9a9497d0da09105df4df/src/core/Oracles/ChainlinkCompositeOracle.sol#L190

Vulnerability details

Impact

The function getPriceAndDecimals() calls the chainlink oracle receiving the latest round of data. It does a safety check by requiring strict equality between answeredInRound and the roundId, but doesn't check if the timestamp is recent.

If there is a problem with chainlink starting a new round and finding consensus on the new value for the oracle (e.g. chainlink nodes abandon the oracle, chain congestion, vulnerability/attacks on the chainlink system) consumers of this contract may continue using outdated stale data (if oracles are unable to submit no new round is started)

Proof of Concept

        (
            uint80 roundId,
            int256 price,
            ,
            ,
            uint80 answeredInRound
        ) = AggregatorV3Interface(oracleAddress).latestRoundData();
        bool valid = price > 0 && answeredInRound == roundId;
        require(valid, "CLCOracle: Oracle data is invalid");

Tools Used

Manual Review

Recommended Mitigation Steps

Consider checking the oracle responses updatedAt value after calling out to chainlinkOracle.latestRoundData() verifying that the result is within an allowed margin of freshness.