Use of deprecated chainlink function

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

When using the latestAnswer() method to fetch the latest asset price, there's a potential issue. This method lacks a mechanism to verify the timeliness of the data, crucial for ensuring accurate and reliable price information in transactions.

Proof of Concept

The ChainlinkPriceOracle contract uses the deprecated AggregatorInterface.latestAnswer() to retrieve the current price instead of the latestroundData() function recommended by Chainlink. The latestAnswer() function might return 0 if no answer is reached, which should be handled appropriately by the caller.

The recommended latestRoundData() function returns additional data which could be used to check whether the answer was reported recently enough.

If latestAnswer() returns 0 for a specific asset, users won't be able to mint rsETH and will receive 0 when withdrawing.

Note

I am aware that this issue has been included in the bot report but still reported this bug because the automated report does not explains the full impact of the finding and hence incorrectly judges it as low.
The C4 docs explicitly states that raising issues from bot reports to a higher severity is fair game, as seen here.

Tools Used

Manual review

Recommended Mitigation Steps

Short term, consider using the latestRoundData function and add checks on the return data with proper revert messages if the price is stale or the round is uncomplete.

Assessed type

Oracle

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #32

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as not a duplicate

[c4-pre-sort]

raymondfam marked the issue as duplicate of #34

[c4-judge]

fatherGoose1 marked the issue as unsatisfactory:
Invalid

[0xbtk]

Hey @fatherGoose1, can you please explain why this is marked as invalid? As per of the C4 docs:

Wardens may use automated tools as a first pass, and build on these findings to identify High and Medium severity issues ("HM issues").

I was aware that this issue has been included in the bot report but still reported this bug because the automated report does not explains the full impact of the finding and hence incorrectly judges it as low.

Thanks for your time and consideration.

[fatherGoose1]

This report does not offer much on top of what the bot has reported. Moreover, this specific finding is widely recognized as OOS given the sheer volume of reports that reference latestAnswer().