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

Insufficient validation of price oracle data #224

Closed
c4-submissions opened this issue Nov 13, 2023 · 6 comments
Closed

Insufficient validation of price oracle data #224

c4-submissions opened this issue Nov 13, 2023 · 6 comments
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 edited-by-warden 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

c4-submissions commented Nov 13, 2023

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 uses the Chainlink aggregation's deprecated getLatestAnswer function to retrieve data. This issue was raised as low severity in the Bot Report, but neither the impact nor the mitigation was elaborated on. Similar issues have been raised as medium severity in previous Code4rena contests.

The latestAnswer function returns the most recent data for the query with no indication of when it was last updated. This could lead to stale values being used, which could impact the accuracy of calculations dependent on the prices returned by the oracle.

Proof of Concept

Tools Used

Manual review.

Recommended Mitigation Steps

latestRoundData should be used instead of latestAnswer. The updatedAt return value should be validated to be within a reasonable window of block.timestamp. Appropriate fallback functionality should be introduced, such as a secondary oracle to query should the primary oracle return a stale value.

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 13, 2023
c4-submissions added a commit that referenced this issue Nov 13, 2023
@c4-pre-sort
Copy link

raymondfam marked the issue as duplicate of #34

@c4-pre-sort
Copy link

raymondfam marked the issue as insufficient quality report

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

raymondfam marked the issue as sufficient quality report

@c4-pre-sort c4-pre-sort added the sufficient quality report This report is of sufficient quality label Nov 17, 2023
@c4-pre-sort c4-pre-sort reopened this 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 edited-by-warden 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

4 participants