code-423n4 / 2023-07-moonwell-findings

1 stars 0 forks source link

Stale price can be used in ChainlinkOracle and ChainlinkCompositeOracle #292

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

ChainlinkOracle and ChainlinkCompositeOracle should use the updatedAt value from the latestRoundData() function to validate the freshness of the price. Otherwise an obsolete price could be used in the protocol which could put its solvency at risk.

lines 97-117 in ChainlinkOracle.sol

    function getChainlinkPrice(
        AggregatorV3Interface feed
    ) internal view returns (uint256) {
        (, int256 answer, , uint256 updatedAt, ) = AggregatorV3Interface(feed)
            .latestRoundData();
        require(answer > 0, "Chainlink price cannot be lower than 0");
        require(updatedAt != 0, "Round is in incompleted state");

        // Chainlink USD-denominated feeds store answers at 8 decimals
        uint256 decimalDelta = uint256(18).sub(feed.decimals());
        // Ensure that we don't multiply the result by 0
        if (decimalDelta > 0) {
            return uint256(answer).mul(10 ** decimalDelta);
        } else {
            return uint256(answer);
        }
    }

lines 180-195 in ChainlinkCompositeOracle.sol

    function getPriceAndDecimals(
        address oracleAddress
    ) public view returns (int256, uint8) {
        (
            uint80 roundId,
            int256 price,
            ,
            ,
            uint80 answeredInRound
        ) = AggregatorV3Interface(oracleAddress).latestRoundData();
        bool valid = price > 0 && answeredInRound == roundId;
        require(valid, "CLCOracle: Oracle data is invalid");
        uint8 oracleDecimals = AggregatorV3Interface(oracleAddress).decimals();

        return (price, oracleDecimals); /// price always gt 0 at this point
    }

Proof of Concept

If the market price of the token flash crashes, and Chainlink's feed does not get updated in time, the smart contract will continue to believe the price of a token is worth more than the market value.

Tools Used

Manual VS code

Recommended Mitigation Steps

Chainlink advise developers to check for the updatedAt before using the price:

Your application should track the latestTimestamp variable or use the updatedAt value from the latestRoundData() function to make sure that the latest answer is recent enough for your application to use it. If your application detects that the reported answer is not updated within the heartbeat or within time limits that you determine are acceptable for your application, pause operation or switch to an alternate operation mode while identifying the cause of the delay.

Assessed type

Oracle

0xSorryNotSorry commented 1 year ago

OOS --> [M‑02] Insufficient oracle validation

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as low quality report

c4-judge commented 1 year ago

alcueca marked the issue as unsatisfactory: Invalid