c4-submissions commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/oracles/ChainlinkPriceOracle.sol#L38

Vulnerability details

Impact

Even if you decide to use the depricated chainlink function, the result should be checked for validity (at least > 0)

Proof of Concept

Currently function doesn't check the return value from Chainlink latestAnswer(), which could be zero or stale. https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/oracles/ChainlinkPriceOracle.sol#L38

Tools Used

Manual Review

Recommended Mitigation Steps

Implement some validations of the received answer and revert if it is not satisfactory:

    function getAssetPrice(address asset) external view onlySupportedAsset(asset) returns (uint256) {
        unit256 price = AggregatorInterface(assetPriceFeed[asset]).latestAnswer();
        if(price == 0){
        revert("Chainlink oracle error!");
        }
        // Add any other validations
        return price;
    }

Assessed type

Oracle

c4-pre-sort commented 11 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 11 months ago

raymondfam marked the issue as duplicate of #34

c4-judge commented 10 months ago

fatherGoose1 marked the issue as unsatisfactory: Invalid

code-423n4 / 2023-11-kelp-findings

Insufficient checks for Chainlink's `latestAnswer()` #340