Renzo - guhu95's results

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Withdraw/WithdrawQueue.sol#L220-L224

Vulnerability details

Cause

Deposit and withdrawal requests can be done immediately with no costs or fees, and both use the current oracle prices and TVL calculation (deposit, and withdraw). Crucially, the withdrawal amount is calculated at withdrawal request submission time instead of at withdrawal claim time. Any small change in either ezETH value or in the price of a collateral token can be exploited at no cost by MEV. Specifically, if the price increases, a deposit is made before the increase, and a withdrawal request immediately after.

Additionally, in case of a supported LST's sudden change in price (for example due to price manipulation, an exploit of that LST, due to consensus layer penalties (slashing), or liquidity issues), external holders of that LST may frontrun the change, deposit the LST into Renzo, and immediately request a withdrawal of another asset (e.g., native ETH). In such situations, Renzo functions as a zero-slippage zero-fees oracle-price-based DEX for LSTs and ETH up to the TVL cap for the affected LST. Zero-slippage zero-fee oracle-price-based designs are notoriously vulnerable to both oracle manipulation and oracle latency attacks if not carefully prevented.

Impact

The newly introduced frontrunning vector, due to incurring only gas fees, and no fee that is proportional to the size of the "trade", allows profitably exploiting most TVL and oracle price changes, and exploiting previously exploitable updates (vie Balancer's usage of getRate()) even more profitably via the new vector.

The impact is that value that otherwise should be distributed to ezETH holders is constantly lost to MEV.

Additionally, ezETH holders lose value due to facilitating asset swaps with no slippage and no fees based on outdated oracle prices.

Proof of Concept

Scenario 1 (MEV, price increase):

1. A transaction that will increase the TVL value without minting or burning ezETH, such as a Chainlink oracle update or EigenLayer rewards withdrawal, is observed.
2. An attacker sandwiches the transaction by depositing asset A right before it (minting ezETH to themselves).
3. The attacker completes the sandwich by submitting a withdrawal request for asset A after the "target" transaction. This sends the ezETH to WithdrawQueue, and credits the attacker with a larger amount of A than was just deposited, corresponding to the updated price of ezETH.
4. The attacker realizes an immediate, same block profit - a larger amount of the same asset, due to the price increase and credit of future withdrawal amount, and no fee being charged.
5. Value that would otherwise be distributed to holders of ezETH was lost to the attacker.

Scenario 2 (malicious):

1. Asset A experiences a sharp price decline due to slashing, exploit, or other factors.
2. An attacker borrows LST asset A from on-chain lending protocols. This is done to reduce the assets available for others to short the price manipulation of the next step.
3. The attacker buys up even more of the LST on CEX and DEX, running up the price of that asset, such that a manipulated price is reported by oracles.
4. The attacker deposits all asset A into Renzo, being credited with an outsized amount of ezETH due to the inflated price of the collateral.
5. The attacker immediately submits a withdrawal request of asset B, which upon conversion uses the inflated value of asset A in Renzo's TVL.
6. 7 days later, upon withdrawal claiming, the attacker withdraws the inflated amount of asset B.

Scenario 3 (MEV, price decrease, zerp-slippage zero-fees DEX):

1. A supported LST experiences a sudden change in price due to factors such as price manipulation, an exploit of that LST, consensus layer penalties (slashing), or liquidity issues.
2. Existing external holders of the LST, or arbitrageurs borrowing the asset, frontrun the transaction that changes the oracle price by depositing the LST into Renzo, and immediately requesting a withdrawal of another asset (e.g., native ETH or another LST). Using Renzo as a zero-slippage oracle-price-based DEX.
3. After the withdrawal delay, the attackers claim their withdrawal, receiving the other asset at the pre-change price, effectively exploiting the ezETH holders.

Tools Used

Manual review.

Recommended Mitigation Steps

The redemption conversion should be performed at both request and claim time. If it results in a lower redeem value, that value should be used for the claim instead of the initial redeem amount. Additionally, a rate limit or a short delay on deposits with similar protection can be added as well.

function claim(uint256 withdrawRequestIndex) external nonReentrant {
    ...
+   // All the code converting from ezETH amount to amountToRedeem as is done in withdraw()

+   if (amountToRedeem < _withdrawRequest.amountToRedeem) {
+       _withdrawRequest.amountToRedeem = amountToRedeem;
+   }
}

Assessed type

MEV

Lines of code

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

Vulnerability details

Cause

WithdrawQueue's getAvailableToWithdraw function may underflow and revert if stETH (or another rebasing LST) experiences a negative rebase. This is because when IERC20(_asset).balanceOf(address(this)) - claimReserve[_asset] is 0 (or nearly 0), a negative rebase will result in stETH balance being lower than claimReserve.

A negative rebase is expected to happen due to slashing, inactivity leaks, or a Lido issue in the case of stETH. Other rebasing LSTs may also expirience a negative rebase during the course of normal operation.

Impact

This will cause the following user flows to revert:

• RestakeManager.deposit
• WithdrawQueue.withdraw

This will also cause the following admin flow to revert:

• OperatorDelegator.completeQueuedWithdrawal

Crucially, because user deposits will revert and OperatorDelegator.completeQueuedWithdrawal will revert, it will not be possible to refill the buffer. This is because refilling the buffer always calls the method to calculate the transfer amount, which will revert for this asset.

For the issue to resolve, unclaimed withdrawals will need to be claimed. However, due to the 7-day delay imposed on unclaiming, this may take up to 7 days. Even then, because a claim transaction may only be submitted by the original withdrawer (msg.sender check), a user may prolong the DoS maliciously or inadvertently by not claiming their withdrawal.

Even if the buffer is not near 0 (when balance and reserve are equal to each other), an attacker may frontrun a negative rebase with a deposit and a withdrawal to "take the buffer" bringing it to 0, such that after their action, the protocol will stop functioning for this asset, possibly until the attacker claims their withdrawal. This will result in a prolonged DoS.

Proof of Concept

1. The buffer for stETH is at (or near) 0 following a withdrawal request "taking" it by increasing the claimReserve, either in the course of normal operations or maliciously.
2. A negative rebase reduces the stETH supply held by the WithdrawQueue.
3. All subsequent calls to the buffer views for stETH will revert, preventing refilling it.
4. User deposits, withdrawals, and admin's claiming of EigenLayer withdrawal (OperatorDelegator.completeQueuedWithdrawal) involving stETH all revert until a sufficient amount of withdrawals is claimed to reduce the claimReserve.

Tools Used

Manual Review

Recommended Mitigation Steps

Check if the subtraction will underflow and report 0 in that case:

    function getAvailableToWithdraw(address _asset) public view returns (uint256) {
        if (_asset != IS_NATIVE) {
-            return IERC20(_asset).balanceOf(address(this)) - claimReserve[_asset];
+            uint256 balance = IERC20(_asset).balanceOf(address(this));
+            return (balance > claimReserve[_asset]) ? balance - claimReserve[_asset] : 0;

Assessed type

DoS

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Withdraw/WithdrawQueue.sol#L217-L233 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Withdraw/WithdrawQueue.sol#L305-L308

Vulnerability details

Cause

The WithdrawQueue contract calculates the withdrawal redemption amount at the time of the withdrawal request but pays out the amount at the time of the claim.

• For ETH and rebasing LSTs like stETH, this means that the withdrawal asset does not earn yield during the delay (as the ETH value of an amount is constant).
• However, for accruing LSTs like wBETH, the withdrawal will continue earning yield during the delay period (because its price increases with the yield).

Impact

For accruing LSTs, this means that yield that can be accrued for ezETH holders, is instead accrued to withdrawals, and lost to the protocol.

This will also create an imbalance in deposit and withdrawal flows because users will prefer to redeem in accruing tokens to avoid losing 7 days of yield.

This, in turn, will skew the asset composition, causing the protocol to accumulate rebasing LSTs over time to reach their TVL cap and revert on subsequent deposits. This will result in an ongoing DoS of deposits for these assets.

While it's possible to keep increasing TVL caps for rebasing LSTs, it is unlikely to be a long term solution, because would not be responsible risk management and would reduce asset diversification.

Proof of Concept

1. Users will deposit all supported LSTs.
2. Users will withdraw predominantly accruing LSTs due to the yield accrued during withdrawal delay.
3. The accruing LSTs' yield will be lost to ezETH holders.
4. The rebasing LSTs will keep accumulating over time in the protocol due to imbalanced flows, hitting TVL caps required for risk management and diversification.
5. Users will not be able to deposit rebasing LSTs when their TVL cap is reached and not increased.

Tools Used

Manual Review

Recommended Mitigation Steps

Calculate the redemption from ezETH to ETH value at the withdrawal request time, but calculate the final LST amount at claim time for all assets. This will keep the ETH value of a withdrawal fixed for the duration of the delay and divert the yield earned to the ezETH holders.

Alternatively, allow the admin to rebalance the asset mixture by swapping assets withdrawn from Eigenlayer before transferring them to the WithdrawQueue.

    function withdraw(uint256 _amount, address _assetOut) external nonReentrant {
	    ...
	    
        // Calculate amount to Redeem in ETH
        uint256 amountToRedeem = renzoOracle.calculateRedeemAmount(
            _amount,
            ezETH.totalSupply(),
            totalTVL
        );

-        // update amount in claim asset, if claim asset is not ETH
-        if (_assetOut != IS_NATIVE) {
-            // Get ERC20 asset equivalent amount
-            amountToRedeem = renzoOracle.lookupTokenAmountFromValue(
-                IERC20(_assetOut),
-                amountToRedeem
-            );
        }

    function claim(uint256 withdrawRequestIndex) external nonReentrant {
	    ...

        WithdrawRequest memory _withdrawRequest = withdrawRequests[msg.sender][
            withdrawRequestIndex
        ];

+        // update amount in claim asset, if claim asset is not ETH
+        if (_withdrawRequest.collateralToken != IS_NATIVE) {
+            // Get ERC20 asset equivalent amount
+            _withdrawRequest.amountToRedeem = renzoOracle.lookupTokenAmountFromValue(
+                IERC20(_withdrawRequest.assetOut),
+                _withdrawRequest.amountToRedeem
+            );
        }

Assessed type

Other

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/RestakeManager.sol#L318

Vulnerability details

Cause

The calculateTVLs function uses uses i (the index of the operator delegator) instead of j (the index of the collateral token) to access the collateralTokens array when calculating the token value of the withdrawal queue balance causing the wrong oracle price to be used for the calculation.

Impact

Because i is 0 when withdrawQueueTokenBalanceRecorded is false, the calculation will always use the price of the first token for all tokens in the withdrawal queue. For all tokens except the first, the TVL calculation will be incorrect and exploitable:

• When OperatorDelegator.completeQueuedWithdrawal is called, it changes the TVL without changing the supply by moving the balance from a correctly accounted location (queuedShares) to an incorrectly accounted one (WithdrawalQueue).
• When RestakeManager.deposit is called, it mints an amount of ezETH based on correct token price but then moves the tokens into the withdrawal queue where they are accounted for with the incorrect price.
• When WithdrawalQueue.claim is called, incorrectly acounted tokens reduce the TVL by an incorrect amount, because the change in shares due to burn does not correspond to the change in TVL due to previously incorrectly accounted tokens that now are transferred out.

Currently, token 0 is wBETH and token 1 is stETH, with a price difference of 4%, meaning that stETH balances in the withdrawal queue are overvalued in the TVL calculation. When more tokens are enabled, larger price differences will be possible. Accumulating tokens (e.g., ANKRETH & rETH) are much more expensive than rebasing ones (stETH, frxETH), with price differences of up to 15%.

This makes the TVL calculation exploitable by frontrunning. A transaction that moves a balance from an incorrectly accounted location to a correctly accounted one changes the TVL, which in turn affects the ezETH share price. This directly changes the deposit and withdrawal prices, as well as the getRate() price value used by the Balancer pool.

Sandwiching this predictable change between either two opposite Balancer trades or a pair of deposit and withdraw actions allows an attacker to steal funds from ezETH holders.

Proof of Concept

Scenario 1:

1. An attacker buys ezETH for ETH via Balancer, which uses getRate() price that uses the TVL calculation.
2. When OperatorDelegator.completeQueuedWithdrawal is called, withdrawing a large amount of stETH, the stETH balance is moved from a correctly accounted location (queuedShares) to an incorrectly accounted one (WithdrawalQueue). This increases the TVL due to using wBETH price for calculating the stETH value added to the WithdrawalQueue.
3. Attacker sells the ezETH for ETH via Balancer, which uses the increased getRate price to realize a risk free profit.

A variety of similar exploits involving both Balancer traders, and deposit and withdrawals are made possible. Both for exploiting the TVL in deposit flow, for the completeQueuedWithdrawal flow, and for the withdrawal flow. They can involve sandwiching large transactions of other users, or using attacker's funds to trigger the deposit and withdrawal flows directly.

Tools Used

Manual review.

Recommended Mitigation Steps

Use the correct index j instead of i when accessing collateralTokens in the totalWithdrawalQueueValue calculation:

totalWithdrawalQueueValue += renzoOracle.lookupTokenValue(
--    collateralTokens[i],
++    collateralTokens[j],
    collateralTokens[j].balanceOf(withdrawQueue)
);

This will ensure that the correct token prices are used for each token in the withdrawal queue.

Assessed type

Oracle

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Withdraw/WithdrawQueue.sol#L206 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Withdraw/WithdrawQueue.sol#L279

Vulnerability details

Cause

The WithdrawQueue contract defines a pausing functionality but does not use it for the user-controlled withdraw and claim functions. This is in contrast to the pausing functionality that is used for deposits and ezETH transfers.

Crucially, during the withdrawal claim() function, ezETH is burned (not transferred), meaning that it cannot be paused using ezETH's pausing functionality as a workaround. This is because ezETH's pausing functionality is limited to transfers and does not pause minting or burning.

Impact

In case of an issue that requires pausing the protocol, for example, during a configuration change, an upgrade, or the mitigation of another issue, funds in the WithdrawQueue will remain vulnerable and may leave the protocol incorrectly, causing a loss of funds to ezETH holders.

Additionally, if ezETH itself is not paused, withdrawal requests will also be callable, potentially taking advantage of possibly incorrectly calculated redemption values, causing loss of value to ezETH holders.

Proof of Concept

N/A

Tools Used

Manual Review

Recommended Mitigation Steps

Use the whenNotPaused modifier on the withdraw() and claim() functions:

-    function withdraw(uint256 _amount, address _assetOut) external nonReentrant {
+    function withdraw(uint256 _amount, address _assetOut) external nonReentrant whenNotPaused {

-    function claim(uint256 withdrawRequestIndex) external nonReentrant {
+    function claim(uint256 withdrawRequestIndex) external nonReentrant whenNotPaused {

Assessed type

Access Control

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/RestakeManager.sol#L289-L310

Vulnerability details

Cause

The calculateTVLs function in the RestakeManager contains two nested loops: one loop for each operator delegator (OD), and an internal loop for each token. In each internal loop:

• The OD's getTokenBalanceFromStrategy function is called for the token, which in turn calls the Eigenlayer strategy for the shares balance.
• The renzoOracle is called for the token value, which in turns calls the respective oracle.
• Additionally, memory is allocated for all the results and intermediate call outputs, incurring high quadratic memory expansion costs.

Due to the the nested loop structure, the gas consumption in the case of several operator delegators, and several tokens used is quadratic: n-OD x n-Tokens. Crucially, this method is called in most user flows: deposits, withdrawals, and Balancer trades.

Impact

An even small number of operators and supported tokens will render the protocol unusable, up to running out of block gas limit.

Currently, with 1 OD and 2 tokens, calculateTVLs consumes approximately 450K gas (see the example transaction gas profiling section).

For reasonable values, such as 12 ODs and 12 tokens (Eigenlayer supports 12 tokens), there would be 144 nested loop iterations instead of just 2 iterations in the current version and configuration. A simple extrapolation, without adding the cost of operations added in the new version, and without estimating the additional quadratic memory expansion costs, would suggest that 144 iterations, instead of the current 2, would cost 32M gas (450K * 144 / 2), exceeding the block gas limit.

However, an earlier limiting factor will be encountered because calculateTVL is called in most user interactions with Renzo: deposit, withdrawal, and Balancer trade. Thus, exceeding even 1M-2M gas on L1 will render most protocol methods practically unusable for users.

Proof of Concept

1. With 1 OD and 2 tokens, calculateTVLs consumes approximately 450K gas (see the example transaction gas profiling section)
2. For reasonable values, such as 12 ODs and 12 tokens currently supported by EigenLayer, there would be 144 nested loop iterations instead of the current 2 .
3. A simple extrapolation, would suggest that 144 iterations, instead of the current 2, would cost 32M gas (450K * 144 / 2), exceeding the block gas limit. Moreover, this is an underestimate, due to not adding the cost of operations added in the new version (relative to the deployed one), and without estimating the additional quadratic memory expansion costs.

Tools Used

Manual Review

Recommended Mitigation Steps

Instead of "pulling" operator delegator token balances, a "pushing" pattern can be used:

• Instead of "pulling" token TVLs from each component, the aggregated TVL sums should be maintained and "pushed" by each OD when its TVL changes (on OD deposits and withdrawals). This way, during admin's OD operations, the RM's TVL cache and each token's total protocol balance is updated.
• Then, total token balances can be used to calculate TVL from RenzoOracle.lookupTokenValues once, which will result in a single loop over the tokens only.
• For enabling chooseOperatorDelegatorForDeposit a Set of ODs that can accept a deposit (are below allocation) can be maintained via "push" updates.

This will remove all loops during the calculation, except for the loop needed to iterate over the token oracles (once).

Alternatively, an off-chain custom TVL oracle can be used, for example, via a custom Chainlink oracle.

Assessed type

DoS

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Bridge/L1/xRenzoBridge.sol#L148-L150

Vulnerability details

Cause

According to Connext documentation, Connext xcall's destination xReceive out-of-gas errors and reverts are not replayable despite transferring funds. While they are recoverable by an admin (as recommended), recovering them without also completing the xReceive logic will leave the lockbox undersupplied with ezETH. However, calling xReceive a second time from the admin is not possible because only the Connext bridge is the allowed caller.

If a revert happens for any reason during the deposit (e.g., a TVL limit is reached or due to any other error condition), or if L1 Connext approved relayer or protocol relayer inadvertently provide insufficient gas, the execute transaction may not revert (reconciled true), and only the inner transaction to the xRenzoBridge contract xReceive will fail.

Impact

As a result, the funds will not be deposited, and ezETH will not be minted and added to the lockbox. This will prevent the ezETH that was minted on L2 from being possible to bridge to L1 and be unwrapped for underlying ezETH. This is because to bridge xezETH from L2, real L1 ezETH must be provided from the lockbox.

Since there is no withdrawal logic on L2 (only on L1), and bridging of ezETH is done through Connext and the lockbox, if this happens, it will cause L2 ezETH not to be possible to bridge back or redeem.

Subsequently, being impossible to bridge to L1 will cause L2 xezETH it to lose value compared to L1 ezETH due to the inability to complete the arbitrage cycle, and equalize prices due to selling.

Proof of Concept

1. A large amount of deposited funds (for which xezETH was minted on L2) was swept from L2 via Connext to L1.
2. The xReceive internal call on L1 failed due to a gas issue or an internal revert.
3. The deposit into Renzo and the subsequent top-up of the lockbox didn't happen (reverted), but the funds were transferred (they are transferred separately ahead of the call) and the bridging marked completed.
4. xezETH bridging from the L2 is limited due to insufficient funds in the lockbox.
5. L2 xezETH depegs relatively to L1 ezETH due to inability to bridge back, causing L2 users to lose value.

Tools Used

Manual Review

Recommended Mitigation Steps

Allow the admin to call xReceive instead of only the Connext bridge. This will allow retrying the reverted portion of the transaction (the whole xReceive sub-call).

-        if (msg.sender != address(connext)) {
+        if (msg.sender != address(connext && !roleManager.isBridgeAdmin(msg.sender)) {
            revert InvalidSender(address(connext), msg.sender);
        }

Assessed type

Other