Lack of slippage and deadline during withdraw and deposit

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Withdraw/WithdrawQueue.sol#L229 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/RestakeManager.sol#L565 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/RestakeManager.sol#L605

Vulnerability details

Impact

When users call withdraw() to burn their ezETH and receive redemption amount in return, there is no provision to provide any slippage & deadline params. This is necessary because the withdraw() function uses values from the oracle and the users may get a worse rate than they planned for. Additionally, the withdraw() function also makes use of calls to calculateTVLs() to fetch the current totalTVL. The calculateTVLs() function makes use of oracle prices too. Note that though there is a MAX_TIME_WINDOW inside these oracle lookup functions, the users are forced to rely on this hardcoded value & can't provide a deadline from their side. These facts are apart from the consideration that users' call to withdraw() could very well be unintentionally/intentionally front-run which causes a drop in totalTVL.
In all of these situations, users receive less than they bargained for and hence a slippage and deadline parameter is necessary.

Similar issue can be seen inside deposit() and depositETH().

Proof of Concept

• https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Withdraw/WithdrawQueue.sol#L229
• https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/RestakeManager.sol#L565
• https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/RestakeManager.sol#L605

Tools Used

Manual review

Recommended Mitigation Steps

Allow users to pass a slippage tolerance value and a deadline parameter while calling these functions.

Assessed type

Other

[jatinj615]

Oracle updates the value every 24 hours. and technically it creates an arbitrage opportunity which will not be beneficial to users as they will arbitraging 1 days reward share and loosing on 7 days rewards due to coolDownPeriod.

[C4-Staff]

CloudEllie marked the issue as primary issue

[c4-judge]

alcueca marked the issue as duplicate of #345

[c4-judge]

alcueca marked the issue as satisfactory

[c4-judge]

alcueca marked the issue as selected for report

[jatinj615]

Hey @alcueca , we need the warden to provide a POC of delta around deposit and withdraw considering we will be implementing slashing pricing mechanism at the time of claim specified in #326.

[s1n1st3r01]

Hey @alcueca

I believe this should be a QA rather than a Med. You need slippage and deadline when the price can be manipulated like in an AMM. Here, since the price of the exchange is always fair because it can’t be moved by however large user operation, but only by changes in oracle price (that is fair by definition), there is no strong case for the same of level of protection as in AMMs. Staking on Lido for example doesn't have slippage or deadline either. https://etherscan.io/address/0x17144556fd3424edc8fc8a4c940b2d04936d17eb#code

[alcueca]

The thing is that Renzo is using market oracles, as stated in #13. These oracles can fluctuate more than the actual exchange rate, due to market forces. In some situations the users might be displeased that their withdrawals rended less value because of a temporary spike in the oracle.

It is quite borderline, because it needs the oracles to have jumps large enough to bother users. However, I do see value to add slippage controls, and let users disable them in volatile conditions when they just want to dump.