code-423n4 / 2024-04-renzo-findings

5 stars 4 forks source link

QA Report #556

Open howlbot-integration[bot] opened 2 months ago

howlbot-integration[bot] commented 2 months ago

See the markdown file with the details of this report here.

CloudEllie commented 1 month ago

Updated link to markdown file

c4-judge commented 1 month ago

alcueca marked the issue as grade-b

Slavchew commented 1 month ago

Hey @alcueca,

Sorry for writing after PJQA, but that's because there are issues that were flagged as valid after it, which are also in our QA report.

Please review them again and upgrade, if think so.

[L-05] is a dup of #282 [L-06] is a possible dup of #13

Sorry one more time.

Slavchew commented 1 month ago

Hey @alcueca,

Sorry for writing again, but why isn't [L-06] a dup of #13 and #424, it is 1:1 the same issue.

Also [L-03] it is the same as #534.

Please review it again and upgrade, if think so.

alcueca commented 1 month ago

13 refers to the fact that there are different sources of price data for stETH, which are market data (from DEXs) and exchange data (from Lido contracts). L-06 is about the long heartbeat of STETH/ETH, which has been declared invalid elsewhere. #424 is about arbitrage due to implying a 1:1 price to several different collaterals, which in fact diverge. Nothing to do with hearbeat.

L-03 upgraded to be a duplicate of #534

Slavchew commented 1 month ago

What about #427, #428, they are very similar to our, it may not be fully duplicate, but it is at least partial-75.

Slavchew commented 1 month ago

@alcueca Here is my final explanation of the situation about L-06.

The issue describes very well that arbitrage opportunities can happen based on the stETH/ETH price feed heartbeat and divination.

The exact same is stated in these reports #429, #428, #427, #424 and #426, which even talk about tokens that will not be used in Renzo.

They are all considered duplicates of #13 and they all talk about a 0.5% price deviation.

Here is what we stated in L-06

Renzo protocol relies on stETH/ETH price feed to convert stETH when used for deposit, withdrawal, etc. But this price feed has too long heartbeat - 24 hours, which can open arbitrage opportunities if the price is not updated accurately. The 24-hour heartbeat and 0.5% deviation threshold means that price can move up to 0.5% or stay flat for 24 hours before triggering a price update, which is unlikely to be reached, but historically it is not impossible, you can check this graph for example. This allows you to deposit at these times to mint more ezETH or withdraw at a better rate.

Here is what is stated in #428

The value of stETH deposited into the protocol is priced using chainlink oracle. The current deviation and heartbeat of stETH/ETH are 0.5% and 86400s. This means stETH price will not be updated it the price doesn't deviate 0.5% in 24 hours. This difference between market price and oracle price can be used to perform arbitrage. The 0.5% deviation might seem less here. But attacker can front run oracle updates to exploit the above scenario which have more deviation thus more profit.

and in #426

The protocol relies solely on Chainlink oracles to get the prices of the collateral deposited into the protocol. The issue is that different LST have different heartbeats and deviation threshold. For example, a deviation threshold of 2% means the feed price can be off by up to 2% as compared to the real tracked price.

Chainlink's STETH/ETH has a heartbeat of 24 hours and a deviation threshold of 0.5%.

RETH/ETH has a heartbeat of 24 hours and a deviation threshold of 2%.

cbETH/ETH has a heartbeat of 24 hours and a deviation threshold of 1%.

These collateral are also updated at different times of the day.

In Renzo, users can deposit one collateral token and immediately call withdraw and exchange their ezETH for another collateral token.

Copied reasoning from similar issue:

If that doesn't show that the L-06 is a valid duplicate of them, I don't know what will.

Bauchibred commented 1 month ago

To add to the above comment @alcueca correct me if I'm wrong, but I believe you're now finalizing all the findings suggesting how the feeds being used would lead to arbitrage under the umbrella of #13.

I'd like to indicate that the sponsor also acknowledged #8 and it's duplicates just like they did for all the other satisfactory arbitrage findings currently under #13, albeit in the case of #8 it's on the ground of how using this feed with a long duration would allow for an arbitrage on deposits, quoting them from here:

... On the other hand, yes there can be minor arbitrage opportunities in stETH deposit, which is an expected behaviour in the protocol. therefore, acknowledging for arbitrage behaviour.

Not to make this comment too long, but to summarize: I believe if you're indeed grouping all the findings that showcase a better implementation for the protocol to curb "feed arbitrage" under #13, then L-06 from here, L-17 from #539, #8, and its duplicates should also be considered under this umbrella. Or maybe they should be treated as partial duplications if you still consider them not direct duplicates. However, if my assessment is wrong about the umbrella under which you're grouping them, then please ignore this comment. Thank you for your time!