# Withdrawals can fail due to deposits reverting in `completeQueuedWithdrawal()` Medium · Code4rena · Liquid restaking · 30th April 2024 Finding M-01 of the Renzo competition. - Protocol: https://code4rena.com/audits/2024-04-renzo - Codebase: https://github.com/0xsimao/2024-04-renzo/tree/519e518f2d8dec9acf6482b84a181e403070d22d - Source: https://github.com/code-423n4/2024-04-renzo-findings/issues/604 --- The [`OperatorDelegator.completeQueuedWithdrawal()`](https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Delegation/OperatorDelegator.sol#L265) function serves to finalize a queued withdrawal consisting of different tokens, sending the withdrawn tokens to the `WithdrawQueue` contract up to the buffer amount, and depositing any excess tokens back into the corresponding EigenLayer strategy. The issue is that the [call](https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Delegation/OperatorDelegator.sol#L168) to `strategyManager.depositIntoStrategy()` used to deposit any amount of excess tokens back into EigenLayer may revert, which would cause the entire `completeQueuedWithdrawal()` transaction to revert. There are a couple reasons why `depositIntoStrategy()` may revert: 1. In `StrategyManager.depositIntoStrategy()`, the number of shares returned by the strategy is [verified](https://github.com/Layr-Labs/eigenlayer-contracts/blob/84ca899c715dbc577af1c8db5965188b061fe464/src/contracts/core/StrategyManager.sol#L297) to be `> 0`. If the strategy mints shares at less than a 1:1 ratio, this can cause the transaction to revert, as the amount being deposited back may be as small as 1 wei. 2. EigenLayer's stETH and wBETH [strategies](https://github.com/Layr-Labs/eigenlayer-contracts?tab=readme-ov-file#strategies) both implement per-transaction and total deposit limits ([stETH](https://github.com/Layr-Labs/eigenlayer-contracts/blob/0139d6213927c0a7812578899ddd3dda58051928/src/contracts/strategies/StrategyBaseTVLLimits.sol#L74-L75), [wBETH](https://github.com/Layr-Labs/eigenlayer-contracts/blob/mainnet/src/contracts/strategies/StrategyBaseTVLLimits.sol#L80-L81)): - If the deposit limit of a strategy has been reached, attempting to complete a queued withdrawal containing any amount over the `WithdrawQueue`'s buffer of the strategy's token will fail. - If the amount being redeposited is large enough, it may surpass the per-deposit limit. This issue forces the `withdrawQueueAdmin` (which is a separate entity from the `nativeEthRestakeAdmin` affected) to set the buffer for the affected token high enough for the full amount being withdrawn to be transferred to the `WithdrawQueue`. This will require a third party to intervene and the protocol to handle in an unintended way by increasing the withdraw buffer for one or more tokens. Furthermore, another entry point that calls `strategyManager.depositIntoStrategy()` and will revert under the same conditions is [`RestakeManager.deposit()`](https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/RestakeManager.sol#L491), the main deposit function for collateral tokens. Also here, the collateral token being deposited is first used to fill the `WithdrawQueue`'s buffer, so the transaction can revert for any of the reasons outlined above. ### Proof of Concept **Scenario A:** 1. The `nativeEthRestakeAdmin` queues a withdrawal for 10 ETH, 10 stETH and 10 wBETH. 2. After the withdrawal delay passes, `completeQueuedWithdrawal()` is called to process the withdrawals. 3. The 10 ETH are forwarded to the deposit queue. 4. Next, 9 stETH are withdrawn and sent to `WithdrawQueue`, filling its stETH buffer. 5. 1 stETH remains and is attempted to be deposited back into the stETH strategy via `strategyManager.depositIntoStrategy()`. 6. However, the stETH strategy has already reached its total deposit limit. 7. The deposit reverts and causes the entire `completeQueuedWithdrawal()` transaction to revert. **Scenario B:** 1. A user calls `RestakeManager.deposit()` to deposit 1 wBETH. 2. All wBETH but 1 wei is used to fill the wBETH buffer in the `WithdrawQueue`. 3. The remaining 1 wei is then attempted to be deposited into the wBETH strategy via `strategyManager.depositIntoStrategy()`. 4. However, it is converted into `0` shares by the strategy. 5. `StrategyManager` verifies that the number of shares minted by the strategy is greater than `0`. 6. Since `0` shares were minted, this check fails. 7. The `strategyManager.depositIntoStrategy()` call reverts. 8. This causes the entire `RestakeManager.deposit()` transaction to revert due to no fault of the user's. ### Recommended Mitigation Steps Consider catching any reverts when depositing excess tokens into strategies. In order to ensure that the excess tokens remain in the system and are accounted for in the TVL, the best option may be to send them to the `WithdrawQueue` in the catch clause, regardless of whether the buffer for the given token is already full. --- Related findings: - [Wrong reward distribution between early and late depositors because of the late `syncRewards()` call in the cycle, `syncReward()` logic should be executed in each withdraw or deposits (without reverting)](https://0xsimao.com/findings/gogopool-reward-rewards-withdraw-deposits): GoGoPool - [Halted withdrawals in BatchOut:withdrawFulfill() due to tokens transfer() reverting on 0 transfer amount](https://0xsimao.com/findings/clip-finance-i-halted-withdrawals-withdraw-transfer): Clip Finance Strategies - [Hints on withdrawal may fail as the ICR changes between claiming collateral surplus and repaying debt](https://0xsimao.com/findings/beraborrow-i-hints-withdrawal-collateral-debt): Beraborrow Managed Dens - [Admin will not be able to only pause deposits in the `Vault` due to incorrect check leading to DoSed withdrawals](https://0xsimao.com/findings/cork-protocol-able-pause-deposits-withdrawals): Cork Protocol