Users could game oracle price deviation

[c4-submissions]

Lines of code

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

Vulnerability details

Summary

Prices returned from Chainlink oracles have different conditions to update the reported values, which can be abused by

Impact

Prices for the different LST assets supported in the Kelp protocol are obtained from a Chainlink oracle. The data feeds for each one can be found here:

• https://data.chain.link/ethereum/mainnet/crypto-eth/steth-eth
• https://data.chain.link/ethereum/mainnet/crypto-eth/cbeth-eth
• https://data.chain.link/ethereum/mainnet/crypto-eth/reth-eth

Chainlink price feeds follow different rules in order to update their prices. There are essentially two conditions that could trigger a price change, a heartbeat and a price deviation. A heartbeat updates the feed if the configured time since last update has elapsed, and the price deviation triggers the update when the price has moved above a configured percentage.

In all three cases, the heartbeat is the same (24 hours), however the price deviation target is not. stETH has a percentage of 0.5%, cbETH of 1% and rETH of 2%. This means that there are big differences between how fast an oracle is updated between the different assets.

As deposits in the protocol can be done using any assets, this means that a user could choose to deposit a particular asset knowing that the current on-chain price is deviated from the actual price, which can go as far as 2% for the rETH case. Note also that this can be used in a front-run fashion before the price of an oracle is updated on-chain, reducing the uncertainty to almost none in order to take advantage of a big price movement.

Similarly, since feeds have different triggers, a user could use this to their advantage to deposit the asset that is currently most deviated between all three. As participation in the protocol is measured by a share of RSETH, this means that the user's share will also include the other two assets, gaining an advantage in value due to the mispriced deposit asset.

Currently, there is no way to withdraw assets from the protocol, but this could eventually lead to a situation where a user deposits one asset before the price movement, and then withdraws after the price has impacted, or simply withdraws any other asset from the pool which has a more recent price update.

Recommendation

As the protocol is currently relying on Chainlink oracles, it is difficult to provide a recommendation using solely these price feeds. A potential solution would be to factor different oracles, such as a TWAP or other on-chain price sources.

Assessed type

Oracle

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #609

[c4-judge]

fatherGoose1 marked the issue as unsatisfactory:
Invalid

[romeroadrian]

@fatherGoose1 I really don't think this issue is correctly marked as a duplicate of #609. I think this is more on the line of #584.

[c4-judge]

fatherGoose1 marked the issue as duplicate of #584

[c4-judge]

fatherGoose1 marked the issue as satisfactory

[c4-judge]

fatherGoose1 changed the severity to 3 (High Risk)

[d3e4]

@fatherGoose1 This report states that the price oracle might be inaccurate. This is not sufficient for #584. The only thing even related to #584 is "As participation in the protocol is measured by a share of RSETH, this means that the user's share will also include the other two assets, gaining an advantage in value due to the mispriced deposit asset." There is not enough information here to arrive at the issue in #584.

"Currently, there is no way to withdraw assets from the protocol, but this could eventually lead to a situation where a user deposits one asset before the price movement, and then withdraws after the price has impacted, or simply withdraws any other asset from the pool which has a more recent price update." seems to suggest the issue described in #443, which is about the fact that you can predict the market at the moment of a price update, but unrelated to #584

[romeroadrian]

@fatherGoose1 This report states that the price oracle might be inaccurate. This is not sufficient for #584. The only thing even related to #584 is "As participation in the protocol is measured by a share of RSETH, this means that the user's share will also include the other two assets, gaining an advantage in value due to the mispriced deposit asset." There is not enough information here to arrive at the issue in #584.

"Currently, there is no way to withdraw assets from the protocol, but this could eventually lead to a situation where a user deposits one asset before the price movement, and then withdraws after the price has impacted, or simply withdraws any other asset from the pool which has a more recent price update." seems to suggest the issue described in #443, which is about the fact that you can predict the market at the moment of a price update, but unrelated to #584

Nice to see you here d, I'm clearly not stating that the price oracle might be inaccurate, I'd recommend reading the issue again.

[d3e4]

@romeroadrian What is your point? Do you mean that what you stated is contrary to "the price oracle might be inaccurate" (in which case you are wrong) or that you stated something else (what?)?

[romeroadrian]

@romeroadrian What is your point? Do you mean that what you stated is contrary to "the price oracle might be inaccurate" (in which case you are wrong) or that you stated something else (what?)?

I mean that you summarized my report using those words which is not even close to the reported issue. I encourage you to read it again.

[d3e4]

@romeroadrian Please be specific about how you think I have misread your report. A dodging encouragement to read it again will not help anyone.

I did not summarise your report by those words. This was the first part of your report. I then mentioned the two other things in your report. If you think I missed something important, be explicit about what it is.

[romeroadrian]

I did not summarise your report by those words. This was the first part of your report.

I think you are really confused here. Those are your words, I literally quoted them. That's not part of my report.

[d3e4]

You are abusing a literal parsing of what I wrote. I am sure you understand what I meant.

Are you unable to be explicit and specific about what I missed?