Renzo - jokr's results

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Delegation/OperatorDelegator.sol#L327

Vulnerability details

Summary

Instead of passing token address, address of current contract is passed while checking if token has queued shares queuedShares[address(this)] == 0 in getTokenBalanceFromStrategy leading to incorrect TVL

Vulnerability detail

The protocol enables users to deposit LSTs which are staked in eigen layer. To get the amount of staked tokens in the Eigen layer the OperatorDelegator uses a function called getTokenBalanceFromStrategy.

    function getTokenBalanceFromStrategy(IERC20 token) external view returns (uint256) {
        return
            queuedShares[address(this)] == 0
                ? tokenStrategyMapping[token].userUnderlyingView(address(this))
                : tokenStrategyMapping[token].userUnderlyingView(address(this)) +
                    tokenStrategyMapping[token].sharesToUnderlyingView(
                        queuedShares[address(token)]
                    );
    }

Accounting for any queued tokens is being handled using queuedShares. So if any tokens are queued, the total staked tokens will be sum of tokens staked to strategy and queued tokens.

tokenStrategyMapping[token].userUnderlyingView(address(this)) +
      tokenStrategyMapping[token].sharesToUnderlyingView(
          queuedShares[address(token)]
      )

But the check to handle if there are any queued amount for the token is incorrect. Instead of checking the queuedShares of a token, its checking the queued shares for current contract address which will always return 0

  queuedShares[address(this)] == 0

Due to this, the function would never consider queued shares of the tokens in the TVL. So TVL value will be incorrect if there are any queued shares.

Attacker can profit from this situation by

1. When the tokens are queued for withdrawl, attacker can mint more ezETH than actually possible because of the low TVL value than actual
2. When the withdrawl is complete, the tokens are added to withdraw buffer and remaining are staked back to Eigen layer, in both cases contributing to TVL
3. Now suddenly the TVL is increased, so attacker has more value for his ezETH tokens, thus gaining profit
4. Attacker can repeat this process every time NativeEthRestakeAdmin queues withdrawl of LST tokens

Impact

Incorrect TVL calculation leading to loss of users value

Tools Used

Manual review

Recommended Mitigation Steps

    function getTokenBalanceFromStrategy(IERC20 token) external view returns (uint256) {
        return
-            queuedShares[address(this)] == 0
+            queuedShares[address(token)] == 0
                ? tokenStrategyMapping[token].userUnderlyingView(address(this))
                : tokenStrategyMapping[token].userUnderlyingView(address(this)) +
                    tokenStrategyMapping[token].sharesToUnderlyingView(
                        queuedShares[address(token)]
                    );
    }

Assessed type

Other

Lines of code

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

Vulnerability details

If any of the validators run by the Renzo protocol are slashed, or if AVS operators delegated to are slashed, the slashing losses should be borne by all ezETH token holders. However, it takes time for the slashing losses to reflect in the TVL, as validator slashes need to be proven in Eigen Pod first using a beacon chain state root. During this time, users can front-run the TVL changes and withdraw their funds from Renzo to avoid losses.

To address this issue, a withdrawal delay was introduced. However, the amountToRedeem is currently calculated during the withdrawal initiation itself instead of calculating it in the claim function. So users can still avoid losses by front-running TVL updates.

Proof of Concept

1. Current TVL of the protocol is 100 ETH and there are 100 ezETH in supply.
2. So current exchange rate is 1 ETH/ezETH.
3. Now one of the validator gets slashed by 20 ETH.
4. So current TVL is 8O ETH and exchange rate is 0.8 ETH/ezETH.
5. But the current TVL is still reported to be 100 ETH as it takes time for EigenPodManager to update the podShares of OperatorDelegator as it only reduces podShares once someone proves to the EigenPod that the validator is slashed on beacon chain using beacon state root.
6. So the users can exit the protocol at a higher exchange rate than intended exchange rate.

This creates a race condition, where users capable of exiting earlier can do so without incurring losses.

Impact

Users can avoid validator and AVS slashing losses

Tools Used

Recommended Mitigation Steps

Instead of calculating amountToRedeem in withdraw function itself calculate it in the claim function.

Assessed type

MEV

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Bridge/L2/xRenzoDeposit.sol#L253 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Bridge/L1/xRenzoBridge.sol#L178

Vulnerability details

Summary

The amount of ezETH minted for bridged assets will not be same as xezETH minted on for the assets in L2

Vulnerability detail

Renzo utilizes connext bridge to support staking through L2 chains. When users stake native tokens in L2 chains using xRenzoDeposit the tokens are converted to nextWETH. Corresponding xezETH is minted for the user using the oracle price in L2

    function _deposit(
        uint256 _amountIn,
        uint256 _minOut,
        uint256 _deadline
    ) internal returns (uint256) {
        // calculate bridgeFee for deposit amount
        uint256 bridgeFee = getBridgeFeeShare(_amountIn);
        // subtract from _amountIn and add to bridgeFeeCollected
        _amountIn -= bridgeFee;
        bridgeFeeCollected += bridgeFee;

        // Trade deposit tokens for nextWETH
        uint256 amountOut = _trade(_amountIn, _deadline);
        if (amountOut == 0) {
            revert InvalidZeroOutput();
        }

        // Fetch price and timestamp of ezETH from the configured price feed
        (uint256 _lastPrice, uint256 _lastPriceTimestamp) = getMintRate();

        // Verify the price is not stale
        if (block.timestamp > _lastPriceTimestamp + 1 days) {
            revert OraclePriceExpired();
        }

        // Calculate the amount of xezETH to mint - assumes 18 decimals for price and token
        uint256 xezETHAmount = (1e18 * amountOut) / _lastPrice;

        // Check that the user will get the minimum amount of xezETH
        if (xezETHAmount < _minOut) {
            revert InsufficientOutputAmount();
        }

        // Verify the deadline has not passed
        if (block.timestamp > _deadline) {
            revert InvalidTimestamp(_deadline);
        }

        // Mint xezETH to the user
        IXERC20(address(xezETH)).mint(msg.sender, xezETHAmount);

        // Emit the event and return amount minted
        emit Deposit(msg.sender, _amountIn, xezETHAmount);
        return xezETHAmount;
    }

and bridged to L1. Then xRenzoBridge receives correponding WETH which will be converted to ETH. This ETH is deposited through restakeManager.depositETH and corresonding ezETH shares are minted using the oracle price feed in L1.

But here the minting of ezETH in L1 is happening after certain delay

1. Bridge takes time to process the requests
2. Batch of deposits are combined and being sent through bridge

Due to handling this way if there are any price changes between both minting xezETH on L2 and ezETH in L1, there will be less or more ezETH minted for all xezETH tokens. If the value is less, there will be no enough tokens to claim all xezETH tokens in the protocol

Impact

Protocol goes to insolvent state

Recommended Mitigation Steps

Make sure the amount of ezETH tokens minted in L1 are equal to xezETH minted on L2

Assessed type

Other

Lines of code

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

Vulnerability details

The deposit function of the RestakeManager contract, enables users to deposit LST tokens into the protocol, getting ezETH tokens in return. The function doesn’t have any type of slippage control, this is relevant in the context of the deposit function, since the amount of tokens received by the user is determined by an interaction with an oracle, meaning that the amount received in return may vary indefinitely while the request is waiting to be executed.

Proof of Concept

As can be observed by looking at its parameters and implementation, the deposit function of the RestakeManager contract, doesn’t have any type of slippage protection:

    function deposit(
        IERC20 _collateralToken,
        uint256 _amount,
        uint256 _referralId
    ) public nonReentrant notPaused {
        // Verify collateral token is in the list - call will revert if not found
        uint256 tokenIndex = getCollateralTokenIndex(_collateralToken);

        // Get the TVLs for each operator delegator and the total TVL
        (
            uint256[][] memory operatorDelegatorTokenTVLs,
            uint256[] memory operatorDelegatorTVLs,
            uint256 totalTVL
        ) = calculateTVLs();

        // Get the value of the collateral token being deposited
        uint256 collateralTokenValue = renzoOracle.lookupTokenValue(_collateralToken, _amount);

         .....

        // Calculate how much ezETH to mint
        uint256 ezETHToMint = renzoOracle.calculateMintAmount(
            totalTVL,
            collateralTokenValue,
            ezETH.totalSupply()
        );

        // Mint the ezETH
        ezETH.mint(msg.sender, ezETHToMint);

    }

As can be observed, this function uses renzoOracle.lookupTokenValue which interacts with oracle to determine the value of deposited collateral token value and uses that value to determine how many tokens to mint.

Meaning that the user has no way to predict how many ezETH they will get back at the moment of minting, as the price could be updated while the request is in the mempool.

Impact

User has no slippage control and can incur indefinite amount of slippage.

Recommended Mitigation Steps

An additional parameter could be added to the deposit function, to let users decide the minimum amount of tokens to be received, with a relative check after minting.

Assessed type

Oracle

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/RestakeManager.sol#L546-L556 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/RestakeManager.sol#L562 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Delegation/OperatorDelegator.sol#L290-L304

Vulnerability details

When a user deposits funds into the Renzo protocol, if the withdraw buffer isn't full, it's filled first. Any remaining amount is then deposited or queued for deposit into the Eigen Layer. However, if the entire deposited amount is used to fill the withdraw buffer, the operatorDelegator.deposit function is called with a zero amount, triggering a revert due to the zero amount check in the deposit function.

    function deposit(
        IERC20 token,
        uint256 tokenAmount
    ) external nonReentrant onlyRestakeManager returns (uint256 shares) {
        // audit-issue This will revert if tokenAmount is zero
        if (address(tokenStrategyMapping[token]) == address(0x0) || tokenAmount == 0)
            revert InvalidZeroInput();

        // Move the tokens into this contract
        token.safeTransferFrom(msg.sender, address(this), tokenAmount);

        return _deposit(token, tokenAmount);
    }

Similarly, if the buffer is already full, bufferToFill is calculated to be zero. This results in depositQueue.fillERC20withdrawBuffer reverting due to the zero amount check as well.

        if (bufferToFill > 0) {
            bufferToFill = (_amount <= bufferToFill) ? _amount : bufferToFill;
            // update amount to send to the operator Delegator
            _amount -= bufferToFill;

            // safe Approve for depositQueue
            _collateralToken.safeApprove(address(depositQueue), bufferToFill);

            // fill Withdraw Buffer via depositQueue
            // @audit-issue this will revert if bufferToFill is zero
            depositQueue.fillERC20withdrawBuffer(address(_collateralToken), bufferToFill);
        }

The same issue occurs in OperatorDelegator when filling the buffer.

Impact

Deposit function will revert almost every case

Proof of Concept

1. Let's say the stETH collateral token has a buffer deficit of 5 tokens.
2. A user tried to deposit 5 stETH to the protocol so all of his funds will be used to fill the buffer itself.
3. So operatorDelegator.deposit will be called with zero amount which will revert due to zero amount check.

Recommended Mitigation Steps

Only call fillERC20withdrawBuffer and deposit functions if the amount is greater than zero.

        if (bufferToFill > 0) {
            bufferToFill = (_amount <= bufferToFill) ? _amount : bufferToFill;
            // update amount to send to the operator Delegator
            _amount -= bufferToFill;

            // safe Approve for depositQueue
            // @audit-issue safe approve shouldn't be used
            _collateralToken.safeApprove(address(depositQueue), bufferToFill);

            // fill Withdraw Buffer via depositQueue
-            depositQueue.fillERC20withdrawBuffer(address(_collateralToken), bufferToFill);
+            if(bufferToFill > 0) {
+                depositQueue.fillERC20withdrawBuffer(address(_collateralToken), bufferToFill);
+            }

                
        }

Assessed type

DoS

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Oracle/RenzoOracle.sol#L92

Vulnerability details

Summary

The discrepancy between ezETH/ETH oracle price and market price can be used for arbitrage opportunities

Vulnerability detail

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

Consider this scenario

1. Lets say the price of 1 stETH/ETH is 1 in oracle. And the market price of stETH/ETH is also 1
2. Now if the market price of stETH/ETH becomes 0.996. But as the deviation doesn't cross 0.5% the price will not get updated.
3. So an attacker with lot of upfront capital finds this arbitrage opportunity to buy stETH for market price in an AMM
4. Lets say has 100 ETH. He gets 100.4 stETH for the market price
5. He then deposits the 100.4 stETH in renzo. The price in renzo is still 1, so he gets 100.4 value of ETH (100.4 ETH value worth of ezETH is minted)
6. Attacker gets profit of 0.4 ETH value from 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. For ex: if the oracle price for stETH/ETH is 1 and it changes to 0.98, now the attacker can front run the oracle update to perform the above attack. Here attacker used 2% deviation to get more profit.

The delay implemented for withdraws can gaurd against using a flash loan to perform this attack. But its still possible for attacker to perform arbitrage using upfront capital

Refer to this for more info https://medium.com/angle-protocol/angle-research-series-part-1-oracles-and-front-running-d75184abc67

Impact

Attacker can make profit using arbitrage opportunity.

Recommended Mitigation Steps

Consider implementing a deposit fee to mitigate against arbitrage opportunities. Ideally, this fee would be larger than the oracle's maximum price deviation so that it is not possible to profit from arbitrage.

Assessed type

Oracle

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Delegation/OperatorDelegator.sol#L338

Vulnerability details

Summary

Beacon chain rewards directed to the Eigen pod are not currently added into TVL calculations. Similarly, rewards held in the Delayed Withdrawal Router are also excluded from TVL calculations, contributing to the discrepancy in the TVL value.

Vulnerability detail

Partial withdrawals occur automatically on the beacon chain when a validator's effective balance exceeds 32 ETH, such as when receiving Beacon chain rewards. The verifyAndProcessWithdrawals function can be utilized to verify these withdrawals, enabling the Pod Owner to withdraw excess ETH via DelayedWithdrawalRouter.createDelayedWithdrawal.

Later, the Pod Owner or anyone can complete the withdrawal after a delay using DelayedWithdrawalRouter.claimDelayedWithdrawals. Currently the delay is set to 14 hours. The rewards withdrawn are subsequently utilized to fill the withdrawal buffer. Any remaining rewards wait in the deposit queue to be restaked.

However, these rewards are presently excluded from the Total Value Locked (TVL) calculation from the moment they enter the Eigen pod until they are transferred to the OperatorDelegator. Essentially, the rewards are not factored into the TVL while they reside in either the Eigen pod or the DelayedWithdrawalRouter. This discrepancy results in an inaccurate TVL value during this period.

Proof of concept

1. Prior to rewards being withdrawn to the protocol, whether residing in the Eigen pod or the Delayed Withdraw Router, an attacker can exploit the situation by minting more ezETH than actually feasible due to the undervaluation of TVL, as rewards are not added to it.

2. Upon claiming the rewards after the delay, the tokens are allocated to the withdrawal buffer, while the remaining rewards will wait to be staked back into the Eigen layer. In both scenarios, the TVL value increases.

3. This sudden surge in TVL results in the attacker having inflated value for their ezETH tokens, thereby yielding profits.

4. The attacker can repeat this cycle by minting ezETH prior to the rewards being claimed.

Impact

Incorrect calculation of TVL leading to loss of users

Recommended Mitigation Steps

TVL should account for rewards held in both Eigen pod and Delayed withdraw router.

ETH rewards in Eigen pod can be accounted using

 beacon chain rewards = address(eigenPod).balance -withdrawableRestakedExecutionLayerGwei

/// @notice the amount of execution layer ETH in this contract that is staked in EigenLayer (i.e. withdrawn from the Beacon Chain but not from EigenLayer),
uint64 public withdrawableRestakedExecutionLayerGwei;

While ETH in delayed withdraw router can be accounted using getUserDelayedWithdrawals function in DelayerWithdrawalRouter contract of EigenLayer.

Assessed type

Other