code-423n4 / 2021-08-notional-findings

3 stars 0 forks source link

Missing validation on latestRoundData #92

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

a_delamo

Vulnerability details

On ExchangeRate.sol, we are using latestRoundData, but there are no validations that the data is not stale.

The current code is:

            (
                /* uint80 */,
                rate,
                /* uint256 */,
                /* uint256 */,
                /* uint80 */
            ) = AggregatorV2V3Interface(rateOracle).latestRoundData();
            require(rate > 0, "ExchangeRate: invalid rate");

But is missing the checks to validate the data is stale

(roundId, rawPrice,, updatedAt, answeredInRound) = AggregatorV2V3Interface(rateOracle).latestRoundData();
require(rawPrice > 0, "Chainlink price <= 0");
require(updateTime != 0, "Incomplete round");
require(answeredInRound >= roundId, "Stale price");

More information: https://docs.chain.link/docs/faq/#how-can-i-check-if-the-answer-to-a-round-is-being-carried-over-from-a-previous-round

ghoul-sol commented 3 years ago

This was a tough one. I thought about it for a while and after reviewing chainlink data I end up in the conclusion that this deserves medium risk. It's not uncommon for chainlink prices to be above 1h old and in the time of big price movements, 1h price lag can significantly influence the protocol.