the `getChainlinkPrice()` function calling the latestRoundData without using the try/catch to avoid bad possible scenario

[code423n4]

Lines of code

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

Vulnerability details

Impact

Call to latestRoundData could potentially revert and make it impossible to query any prices. the getChainlinkPrice function should use try/catch to avoid the case of the getChainlinkPrice function revert and cause dos/block the system.

Proof of Concept

the function getChainlinkPrice using the latestRoundData without using the try/catch that is recommended to use when calling the latestRoundData:

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);
        }
    }

in this case to prevent bade scenario like denial of service and function block then it is recommended to query Chainlink price feeds using a defensive approach with Solidity’s try/catch structure. In this way, if the call to the price feed fails, the caller contract is still in control and can handle any errors safely and explicitly

Tools Used

manual review

Recommended Mitigation Steps

Surround the call to `latestRoundData() with try/catch instead of calling it directly.

Assessed type

Oracle

[0xSorryNotSorry]

The submission does not provide any demonstration of the issue and the reasoning.

[c4-pre-sort]

0xSorryNotSorry marked the issue as low quality report

[alcueca]

Lack of proof. Chainlink actually tends to return wrong data instead of reverting, which would be the safe scenario.

[c4-judge]

alcueca marked the issue as unsatisfactory: Insufficient proof