code423n4 commented 12 months ago

Lines of code

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

Vulnerability details

Impact

Even tough the dev is checking if the data is stale or not some valid prices could still be considered as invalid because of this line: answeredInRound == roundId, sometimes the answeredInRound might be greater than roundId and would still be valid but in this case would be considered as stale.

Proof of Concept

190:        bool valid = price > 0 && answeredInRound == roundId;

using answeredInRound == roundId will not cover the cases where anwseredInRound is still valid and not stale but is greater than roundId, which happens in Chainlink.

Tools Used

Manual review

Recommended Mitigation Steps

use answeredInRound >= roundId instead, it covers more scenarios, including unexpected ones where answeredInRound might be greater than roundId, ensuring data is not stale.