Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecated oracle function latestAnswer() could bring fund loss #40

Closed
c4-submissions opened this issue Nov 11, 2023 · 6 comments
Closed
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-843 sufficient quality report This report is of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards

Comments

@c4-submissions
Copy link
Contributor

Lines of code

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

Vulnerability details

Impact

The function ChainlinkPriceOracle.getAssetPrice() is used to return important values used throughout the contract, the staleness of the Chainlink return values will lead to wrong calculation of the asset and other unexpected behavior.

Proof of Concept

The function ChainlinkPriceOracle.getAssetPrice() uses Chainlink's deprecated latestAnswer function (https://docs.chain.link/data-feeds/api-reference/), this function also does not guarantee that the price returned by the Chainlink price feed is not stale and there is no additional checks to ensure that the return values are valid.

    function getAssetPrice(address asset) external view onlySupportedAsset(asset) returns (uint256) {
        return AggregatorInterface(assetPriceFeed[asset]).latestAnswer();
    }

Tools Used

Vscode

Recommended Mitigation Steps

The latestRoundData function should be used instead of the deprecated latestAnswer function and add sufficient checks to ensure that the pricefeed is not stale.

(uint80 roundId, int256 assetChainlinkPriceInt, , uint256 updatedAt, uint80 answeredInRound) = IPrice(_chainlinkFeed).latestRoundData();
            require(answeredInRound >= roundId, "price is stale");
            require(updatedAt > 0, "round is incomplete");

Assessed type

Oracle

@c4-submissions c4-submissions added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Nov 11, 2023
c4-submissions added a commit that referenced this issue Nov 11, 2023
@c4-pre-sort
Copy link

raymondfam marked the issue as insufficient quality report

@c4-pre-sort c4-pre-sort added the insufficient quality report This report is not of sufficient quality label Nov 15, 2023
@c4-pre-sort
Copy link

raymondfam marked the issue as duplicate of #34

@c4-pre-sort
Copy link

raymondfam marked the issue as sufficient quality report

@c4-pre-sort c4-pre-sort added sufficient quality report This report is of sufficient quality and removed insufficient quality report This report is not of sufficient quality labels Nov 17, 2023
@c4-pre-sort
Copy link

raymondfam marked the issue as not a duplicate

@c4-pre-sort
Copy link

raymondfam marked the issue as duplicate of #843

@c4-judge
Copy link
Contributor

fatherGoose1 marked the issue as unsatisfactory:
Invalid

@c4-judge c4-judge added the unsatisfactory does not satisfy C4 submission criteria; not eligible for awards label Nov 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-843 sufficient quality report This report is of sufficient quality unsatisfactory does not satisfy C4 submission criteria; not eligible for awards
Projects
None yet
Development

No branches or pull requests

3 participants