Renzo - crypticdefense's results

Lines of code

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

Vulnerability details

Impact

The protocol mentions that stETH and wBETH can be used as collateral tokens. In some instances, the prices of these assets may drop (i.e, large withdrawal). A user can avoid the loss of the price drop by depositing the asset into the Renzo protocol before the price drop and immediately request a withdrawal by specifying a different asset. Users of Renzo will suffer because now they have to deal with the price drop of the asset.

Proof of Concept

The prices of stETH and wBETH may experience price drops:

https://coinmarketcap.com/currencies/steth/ https://coinmarketcap.com/currencies/wrapped-beacon-eth/

Let's say an attacker observes that the price of stETH will drop soon because of a large withdrawal. The attacker can deposit their stETH into Renzo as collateral for ezETH tokens.

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

    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);

        // Enforce TVL limit if set, 0 means the check is not enabled
        if (maxDepositTVL != 0 && totalTVL + collateralTokenValue > maxDepositTVL) {
            revert MaxTVLReached();
        }

        // Enforce individual token TVL limit if set, 0 means the check is not enabled
        if (collateralTokenTvlLimits[_collateralToken] != 0) {
            // Track the current token's TVL
            uint256 currentTokenTVL = 0;

            // For each OD, add up the token TVLs
            uint256 odLength = operatorDelegatorTokenTVLs.length;
            for (uint256 i = 0; i < odLength; ) {
                currentTokenTVL += operatorDelegatorTokenTVLs[i][tokenIndex];
                unchecked {
                    ++i;
                }
            }

            // Check if it is over the limit
            if (currentTokenTVL + collateralTokenValue > collateralTokenTvlLimits[_collateralToken])
                revert MaxTokenTVLReached();
        }

        // Determine which operator delegator to use
        IOperatorDelegator operatorDelegator = chooseOperatorDelegatorForDeposit(
            operatorDelegatorTVLs,
            totalTVL
        );

        // Transfer the collateral token to this address
        _collateralToken.safeTransferFrom(msg.sender, address(this), _amount);

        // Check the withdraw buffer and fill if below buffer target
        uint256 bufferToFill = depositQueue.withdrawQueue().getBufferDeficit(
            address(_collateralToken)
        );
        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
            depositQueue.fillERC20withdrawBuffer(address(_collateralToken), bufferToFill);
        }

        // Approve the tokens to the operator delegator
        _collateralToken.safeApprove(address(operatorDelegator), _amount);

        // Call deposit on the operator delegator
        operatorDelegator.deposit(_collateralToken, _amount);

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

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

        // Emit the deposit event
        emit Deposit(msg.sender, _collateralToken, _amount, ezETHToMint, _referralId);
    }

Then immediately call WithdrawQueue::withdraw to exchange their ezETH for wBETH: https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Withdraw/WithdrawQueue.sol#L206

 function withdraw(uint256 _amount, address _assetOut) external nonReentrant {
        // check for 0 values
        if (_amount == 0 || _assetOut == address(0)) revert InvalidZeroInput();

        // check if provided assetOut is supported
        if (withdrawalBufferTarget[_assetOut] == 0) revert UnsupportedWithdrawAsset();

        // transfer ezETH tokens to this address
        IERC20(address(ezETH)).safeTransferFrom(msg.sender, address(this), _amount);

        // calculate totalTVL
        (, , uint256 totalTVL) = restakeManager.calculateTVLs();

        // 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
            );
        }

        // revert if amount to redeem is greater than withdrawBufferTarget
        if (amountToRedeem > getAvailableToWithdraw(_assetOut)) revert NotEnoughWithdrawBuffer();

        // increment the withdrawRequestNonce
        withdrawRequestNonce++;

        // add withdraw request for msg.sender
        withdrawRequests[msg.sender].push(
            WithdrawRequest(
                _assetOut,
                withdrawRequestNonce,
                amountToRedeem,
                _amount,
                block.timestamp
            )
        );

        // add redeem amount to claimReserve of claim asset
        claimReserve[_assetOut] += amountToRedeem;

        emit WithdrawRequestCreated(
            withdrawRequestNonce,
            msg.sender,
            _assetOut,
            amountToRedeem,
            _amount,
            withdrawRequests[msg.sender].length - 1
        );
    }

Note that Renzo does not incorporate any checks to ensure that the user can only withdraw the token that they deposited with.

The user receives wBETH from the withdrawal and now users of Renzo have the stETH that will experience a price drop, causing a loss for them.

Tools Used

Manual review.

Recommended Mitigation Steps

Consider only allowing users to withdraw the same token they deposited with.

Assessed type

Token-Transfer

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

Impact

When validator penalties occur, a call to the respective Eigenpod is made via Eigenpod::verifyBalancesUpdates() or Eigenpod::verifyAndProcessWithdrawals().

A user of Renzo protocol can front-run this transaction and immediately call WithdrawalQueue::withdraw to avoid this penalty. This works if the buffer has enough funds to process with withdrawal.

Proof of Concept

As mentioned, a validator can experience penalties and users can know ahead of time by front-running the calls that are made when penalties occur, such as Eigenpod::verifyBalancesUpdates.

The user can proceed to call WithdrawalQueue::withdraw https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Withdraw/WithdrawQueue.sol#L206

   /**
     * @notice  Creates a withdraw request for user
     * @param   _amount  amount of ezETH to withdraw
     * @param   _assetOut  output token to receive on claim
     */
    function withdraw(uint256 _amount, address _assetOut) external nonReentrant {
        // check for 0 values
        if (_amount == 0 || _assetOut == address(0)) revert InvalidZeroInput();

        // check if provided assetOut is supported
        if (withdrawalBufferTarget[_assetOut] == 0) revert UnsupportedWithdrawAsset();

        // transfer ezETH tokens to this address
        IERC20(address(ezETH)).safeTransferFrom(msg.sender, address(this), _amount);

        // calculate totalTVL
        (, , uint256 totalTVL) = restakeManager.calculateTVLs();

        // 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
            );
        }

        // revert if amount to redeem is greater than withdrawBufferTarget
@>      if (amountToRedeem > getAvailableToWithdraw(_assetOut)) revert NotEnoughWithdrawBuffer();

        // increment the withdrawRequestNonce
        withdrawRequestNonce++;

        // add withdraw request for msg.sender
        withdrawRequests[msg.sender].push(
            WithdrawRequest(
                _assetOut,
                withdrawRequestNonce,
                amountToRedeem,
                _amount,
                block.timestamp
            )
        );

        // add redeem amount to claimReserve of claim asset
        claimReserve[_assetOut] += amountToRedeem;

        emit WithdrawRequestCreated(
            withdrawRequestNonce,
            msg.sender,
            _assetOut,
            amountToRedeem,
            _amount,
            withdrawRequests[msg.sender].length - 1
        );
    }

As long as there are enough tokens in the buffer, the user will effectively be able to avoid these penalties and other users will suffer.

Tools Used

Manual Review.

Recommended Mitigation Steps

When users call WithdrawalQueue::withdraw, there is a delay before the user can finalize their withdrawal via WithdrawalQueue::claim. Consider implementing a check there to see if validator experienced any penalties, and proceed to distribute the penalty correctly to the caller.

Assessed type

Context

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Delegation/OperatorDelegator.sol#L265 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Delegation/OperatorDelegator.sol#L446 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Deposits/DepositQueue.sol#L134

Vulnerability details

Impact

Withdrawal in the Renzo protocol works the following way:

The withdrawQueue contract get filled by 3 ways ->
1. New Deposits
2. Daily Rewards Coming in the Protocol.
3. Manual withdrawal from EigenLayer. Permissioned call from OperatorDelegator.
-> If options 1 and 2 are not sufficient to fulfil withdraw requests of Users then admin accounts will manually unstake from EigenLayer periodically through 2 step process (in case of ETH 3 steps) -
OperatorDelegator.queueWithdrawals
2.a. OperatorDelegator.verifyAndProcessWithdrawals (for ETH full withdrawal from EigenLayer which requires proof submission generated Offchain ).
2.b. OperatorDelegator.completeQueuedWithdrawals.

When withdrawing manually from Eigenlayer, the withdrawal is queued from OperatorDelegator. The final step is to call OperatorDelegator::completeQueuedWithdrawals. However, due to an issue regarding access control, this will revert when token is not native ETH. In addition, these tokens cannot be recovered and may be stuck in the OperatorDelegator contract.

Proof of Concept

As mentioned above, the last step when withdrawing from Eigenlayer is to make the following call:

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

    /**
     * @notice  Complete the specified withdrawal,
     * @dev     permissioned call (onlyNativeEthRestakeAdmin)
     * @param   withdrawal  Withdrawal struct
     * @param   tokens  list of tokens to withdraw
     * @param   middlewareTimesIndex  is the index in the operator that the staker who triggered the withdrawal was delegated to's middleware times array
     */
    function completeQueuedWithdrawal(
        IDelegationManager.Withdrawal calldata withdrawal,
        IERC20[] calldata tokens,
        uint256 middlewareTimesIndex
    ) external nonReentrant onlyNativeEthRestakeAdmin {
        uint256 gasBefore = gasleft();
        if (tokens.length != withdrawal.strategies.length) revert MismatchedArrayLengths();

        // complete the queued withdrawal from EigenLayer with receiveAsToken set to true
        delegationManager.completeQueuedWithdrawal(withdrawal, tokens, middlewareTimesIndex, true);

        IWithdrawQueue withdrawQueue = restakeManager.depositQueue().withdrawQueue();
        for (uint256 i; i < tokens.length; ) {
            if (address(tokens[i]) == address(0)) revert InvalidZeroInput();

            // deduct queued shares for tracking TVL
            queuedShares[address(tokens[i])] -= withdrawal.shares[i];

            // check if token is not Native ETH
@>          if (address(tokens[i]) != IS_NATIVE) {
                // Check the withdraw buffer and fill if below buffer target
                uint256 bufferToFill = withdrawQueue.getBufferDeficit(address(tokens[i]));

                // get balance of this contract
                uint256 balanceOfToken = tokens[i].balanceOf(address(this));
                if (bufferToFill > 0) {
                    bufferToFill = (balanceOfToken <= bufferToFill) ? balanceOfToken : bufferToFill;

                    // update amount to send to the operator Delegator
                    balanceOfToken -= bufferToFill;

                    // safe Approve for depositQueue
                    tokens[i].safeApprove(address(restakeManager.depositQueue()), bufferToFill);

                    // fill Withdraw Buffer via depositQueue
@>                  restakeManager.depositQueue().fillERC20withdrawBuffer(
                        address(tokens[i]),
                        bufferToFill
                    );
                }

                // Deposit remaining tokens back to eigenLayer
                if (balanceOfToken > 0) {
                    _deposit(tokens[i], balanceOfToken);
                }
            }
            unchecked {
                ++i;
            }
        }

        // emits the Withdraw Completed event with withdrawalRoot
        emit WithdrawCompleted(
            delegationManager.calculateWithdrawalRoot(withdrawal),
            withdrawal.strategies,
            withdrawal.shares
        );
        // record current spent gas
        _recordGas(gasBefore);
    }

We can see that when address(tokens[i]) != IS_NATIVE a call to restakeManager.depositQueue().fillERC20withdrawBuffer is made.

restakeManager.depositQueue() returns the address of the DepositQueue contract and a call to DepositQueue::fillERC20withdrawBuffer is made:

DepositQueue::fillERC20withdrawBuffer https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Deposits/DepositQueue.sol#L134

    function fillERC20withdrawBuffer(
        address _asset,
        uint256 _amount
@>  ) external nonReentrant onlyRestakeManager {
        if (_amount == 0 || _asset == address(0)) revert InvalidZeroInput();
        // safeTransfer from restake manager to this address
        IERC20(_asset).safeTransferFrom(msg.sender, address(this), _amount);
        // approve the token amount for withdraw queue
        IERC20(_asset).safeApprove(address(withdrawQueue), _amount);
        // call the withdraw queue to fill up the buffer
        withdrawQueue.fillERC20WithdrawBuffer(_asset, _amount);
    }

However, we can clearly see that this is a permissioned call via onlyRestakeManager modifier:

DepositQueue::onlyRestakeManager https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Deposits/DepositQueue.sol#L50

    /// @dev Allows only the RestakeManager address to call functions
    modifier onlyRestakeManager() {
        if (msg.sender != address(restakeManager)) revert NotRestakeManager();
        _;
    }

Since DepositQueue::fillERC20withdrawBuffer was called from the OperatorDelegator contract, that will be the address of msg.sender. We can see here that only the RestakeManager contract can call it:

DepositQueue::setRestakeManager https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Deposits/DepositQueue.sol#L112

    /// @dev Sets the address of the RestakeManager contract
    function setRestakeManager(IRestakeManager _restakeManager) external onlyRestakeManagerAdmin {
        if (address(_restakeManager) == address(0x0)) revert InvalidZeroInput();

@>      restakeManager = _restakeManager;

        emit RestakeManagerUpdated(_restakeManager);
    }

Therefore this call will revert and OperatorDelegator::completeQueuedWithdrawals will revert.

To make matters worse, these tokens may be trapped in the contract:

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

    /**
     * @notice  Recover tokens accidentally sent to EigenPod
     * @dev     Only callable by admin
     * @param   tokenList  .
     * @param   amountsToWithdraw  .
     * @param   recipient  .
     */
    function recoverTokens(
        IERC20[] memory tokenList,
        uint256[] memory amountsToWithdraw,
        address recipient
    ) external onlyNativeEthRestakeAdmin {
        eigenPod.recoverTokens(tokenList, amountsToWithdraw, recipient);
    }

This is the only recovery function but it only recovers tokens accidentally sent to EigenPod, not tokens within OperatorDelegator.

Tools Used

Manual Review.

Recommended Mitigation Steps

Manage the access control of DepositQueue::fillERC20withdrawBuffer to allow calls from the OperatorDelegator contract.

Assessed type

Access Control

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/RestakeManager.sol#L318 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/RestakeManager.sol#L504 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Withdraw/WithdrawQueue.sol#L217 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Oracle/RenzoOracle.sol#L68-L81

Vulnerability details

Impact

RestakeManager::calculateTVLs returns an incorrect value of totalTVL due to an error when accounting for the totalWithdrawalQueueValue. This will impact functions such as RestakeManager::deposit and WithdrawalQueue::withdraw, which will mint/burn an incorrect amount of ezETH from users.

Proof of Concept

/// @dev This function calculates the TVLs for each operator delegator by individual token, total for each OD, and total for the protocol.
    /// @return operatorDelegatorTokenTVLs Each OD's TVL indexed by operatorDelegators array by collateralTokens array
    /// @return operatorDelegatorTVLs Each OD's Total TVL in order of operatorDelegators array
    /// @return totalTVL The total TVL across all operator delegators.
    function calculateTVLs() public view returns (uint256[][] memory, uint256[] memory, uint256) {
        uint256[][] memory operatorDelegatorTokenTVLs = new uint256[][](operatorDelegators.length);
        uint256[] memory operatorDelegatorTVLs = new uint256[](operatorDelegators.length);
        uint256 totalTVL = 0;

        // Iterate through the ODs
        uint256 odLength = operatorDelegators.length;

        // flag for withdrawal queue balance set
        bool withdrawQueueTokenBalanceRecorded = false;
        address withdrawQueue = address(depositQueue.withdrawQueue());

        // withdrawalQueue total value
        uint256 totalWithdrawalQueueValue = 0;

        for (uint256 i = 0; i < odLength; ) {
            // Track the TVL for this OD
            uint256 operatorTVL = 0;

            // Track the individual token TVLs for this OD - native ETH will be last item in the array
            uint256[] memory operatorValues = new uint256[](collateralTokens.length + 1);
            operatorDelegatorTokenTVLs[i] = operatorValues;

            // Iterate through the tokens and get the value of each
            uint256 tokenLength = collateralTokens.length;
            for (uint256 j = 0; j < tokenLength; ) {
                // Get the value of this token

                uint256 operatorBalance = operatorDelegators[i].getTokenBalanceFromStrategy(
                    collateralTokens[j]
                );

                // Set the value in the array for this OD
                operatorValues[j] = renzoOracle.lookupTokenValue(
                    collateralTokens[j],
                    operatorBalance
                );

                // Add it to the total TVL for this OD
                operatorTVL += operatorValues[j];

                // record token value of withdraw queue
                if (!withdrawQueueTokenBalanceRecorded) {
@>                  totalWithdrawalQueueValue += renzoOracle.lookupTokenValue(
@>                      collateralTokens[i],
@>                      collateralTokens[j].balanceOf(withdrawQueue)
                    );
                }

                unchecked {
                    ++j;
                }
            }

            // Get the value of native ETH staked for the OD
            uint256 operatorEthBalance = operatorDelegators[i].getStakedETHBalance();

            // Save it to the array for the OD
            operatorValues[operatorValues.length - 1] = operatorEthBalance;

            // Add it to the total TVL for this OD
            operatorTVL += operatorEthBalance;

            // Add it to the total TVL for the protocol
            totalTVL += operatorTVL;

            // Save the TVL for this OD
            operatorDelegatorTVLs[i] = operatorTVL;

            // Set withdrawQueueTokenBalanceRecorded flag to true
            withdrawQueueTokenBalanceRecorded = true;

            unchecked {
                ++i;
            }
        }

        // Get the value of native ETH held in the deposit queue and add it to the total TVL
        totalTVL += address(depositQueue).balance;

        // Add native ETH help in withdraw Queue and totalWithdrawalQueueValue to totalTVL
@>      totalTVL += (address(withdrawQueue).balance + totalWithdrawalQueueValue);

        return (operatorDelegatorTokenTVLs, operatorDelegatorTVLs, totalTVL);
    }

Let's take a closer look when calculating the token value of withdraw queue totalWithdrawalQueueValue:

        // record token value of withdraw queue
                if (!withdrawQueueTokenBalanceRecorded) {
@>                  totalWithdrawalQueueValue += renzoOracle.lookupTokenValue(
@>                      collateralTokens[i],
@>                      collateralTokens[j].balanceOf(withdrawQueue)
                    );

As you can see, there is an error here. We are always looking up the token value of collateralTokens[i] by passing in the balance of collateralTokens[j].

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

    /// @dev Given a single token and balance, return value of the asset in underlying currency
    /// The value returned will be denominated in the decimal precision of the lookup oracle
    /// (e.g. a value of 100 would return as 100 * 10^18)
    function lookupTokenValue(IERC20 _token, uint256 _balance) public view returns (uint256) {
        AggregatorV3Interface oracle = tokenOracleLookup[_token];
        if (address(oracle) == address(0x0)) revert OracleNotFound();

        (, int256 price, , uint256 timestamp, ) = oracle.latestRoundData();
        if (timestamp < block.timestamp - MAX_TIME_WINDOW) revert OraclePriceExpired();
        if (price <= 0) revert InvalidOraclePrice();

        // Price is times 10**18 ensure value amount is scaled
        return (uint256(price) * _balance) / SCALE_FACTOR;
    }

collateralTokens[i] will not always be the same as collateralTokens[j], therefore this the value returned by this function will be incorrect, and totalWithdrawalQueueValue will be incorrect.

Since we add totalWithdrawalQueueValue to totalTVL:

totalTVL += (address(withdrawQueue).balance + totalWithdrawalQueueValue);

totalTVL value will be incorrect.

Tools Used

Manual Review

Recommended Mitigation Steps

Ensure that the correct index is used when calculating totalWithdrawalQueueValue:

        // record token value of withdraw queue
                if (!withdrawQueueTokenBalanceRecorded) {
                    totalWithdrawalQueueValue += renzoOracle.lookupTokenValue(
-                       collateralTokens[i],
+                       collateralTokens[j],
                        collateralTokens[j].balanceOf(withdrawQueue)
                    );

Assessed type

Error

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/RestakeManager.sol#L491 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/RestakeManager.sol#L274

Vulnerability details

Impact

Both depositing and withdrawing calculate the totalTVL that is used to determine how much ezETH to mint and how many tokens to send to the caller.

The problem is that the tvl can change prior to the execution of deposit and withdraw functions (while it's in the mempool). Users may not receive the expected amount of tokens.

Proof of Concept

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

    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);

        // Enforce TVL limit if set, 0 means the check is not enabled
        if (maxDepositTVL != 0 && totalTVL + collateralTokenValue > maxDepositTVL) {
            revert MaxTVLReached();
        }

        // Enforce individual token TVL limit if set, 0 means the check is not enabled
        if (collateralTokenTvlLimits[_collateralToken] != 0) {
            // Track the current token's TVL
            uint256 currentTokenTVL = 0;

            // For each OD, add up the token TVLs
            uint256 odLength = operatorDelegatorTokenTVLs.length;
            for (uint256 i = 0; i < odLength; ) {
                currentTokenTVL += operatorDelegatorTokenTVLs[i][tokenIndex];
                unchecked {
                    ++i;
                }
            }

            // Check if it is over the limit
            if (currentTokenTVL + collateralTokenValue > collateralTokenTvlLimits[_collateralToken])
                revert MaxTokenTVLReached();
        }

        // Determine which operator delegator to use
        IOperatorDelegator operatorDelegator = chooseOperatorDelegatorForDeposit(
            operatorDelegatorTVLs,
            totalTVL
        );

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

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

        // Emit the deposit event
        emit Deposit(msg.sender, _collateralToken, _amount, ezETHToMint, _referralId);
    }

The values returned by calculateTVLs() determines how much ezETH to mint to the user.

When users withdraw, they can call WithdrawQueue::withdraw https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Withdraw/WithdrawQueue.sol#L206

    function withdraw(uint256 _amount, address _assetOut) external nonReentrant {
        // check for 0 values
        if (_amount == 0 || _assetOut == address(0)) revert InvalidZeroInput();

        // check if provided assetOut is supported
        if (withdrawalBufferTarget[_assetOut] == 0) revert UnsupportedWithdrawAsset();

        // transfer ezETH tokens to this address
        IERC20(address(ezETH)).safeTransferFrom(msg.sender, address(this), _amount);

        // calculate totalTVL
@>      (, , uint256 totalTVL) = restakeManager.calculateTVLs();

        // 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
            );
        }

        // revert if amount to redeem is greater than withdrawBufferTarget
        if (amountToRedeem > getAvailableToWithdraw(_assetOut)) revert NotEnoughWithdrawBuffer();

        // increment the withdrawRequestNonce
        withdrawRequestNonce++;

        // add withdraw request for msg.sender
        withdrawRequests[msg.sender].push(
            WithdrawRequest(
                _assetOut,
                withdrawRequestNonce,
                amountToRedeem,
                _amount,
                block.timestamp
            )
        );

        // add redeem amount to claimReserve of claim asset
        claimReserve[_assetOut] += amountToRedeem;

        emit WithdrawRequestCreated(
            withdrawRequestNonce,
            msg.sender,
            _assetOut,
            amountToRedeem,
            _amount,
            withdrawRequests[msg.sender].length - 1
        );
    }

totalTVL returned from calculateTVLs() determines the amountToRedeem.

Let's take a look at RestakeManager::calculateTVLs: https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/RestakeManager.sol#L274

 function calculateTVLs() public view returns (uint256[][] memory, uint256[] memory, uint256) {
        uint256[][] memory operatorDelegatorTokenTVLs = new uint256[][](operatorDelegators.length);
        uint256[] memory operatorDelegatorTVLs = new uint256[](operatorDelegators.length);
        uint256 totalTVL = 0;

        // Iterate through the ODs
        uint256 odLength = operatorDelegators.length;

        // flag for withdrawal queue balance set
        bool withdrawQueueTokenBalanceRecorded = false;
        address withdrawQueue = address(depositQueue.withdrawQueue());

        // withdrawalQueue total value
        uint256 totalWithdrawalQueueValue = 0;

        for (uint256 i = 0; i < odLength; ) {
            // Track the TVL for this OD
            uint256 operatorTVL = 0;

            // Track the individual token TVLs for this OD - native ETH will be last item in the array
            uint256[] memory operatorValues = new uint256[](collateralTokens.length + 1);
            operatorDelegatorTokenTVLs[i] = operatorValues;

            // Iterate through the tokens and get the value of each
            uint256 tokenLength = collateralTokens.length;
            for (uint256 j = 0; j < tokenLength; ) {
                // Get the value of this token

                uint256 operatorBalance = operatorDelegators[i].getTokenBalanceFromStrategy(
                    collateralTokens[j]
                );

                // Set the value in the array for this OD
                operatorValues[j] = renzoOracle.lookupTokenValue(
                    collateralTokens[j],
                    operatorBalance
                );

                // Add it to the total TVL for this OD
                operatorTVL += operatorValues[j];

                // record token value of withdraw queue
                if (!withdrawQueueTokenBalanceRecorded) {
                    totalWithdrawalQueueValue += renzoOracle.lookupTokenValue(
                        collateralTokens[i],
                        collateralTokens[j].balanceOf(withdrawQueue)
                    );
                }

                unchecked {
                    ++j;
                }
            }

            // Get the value of native ETH staked for the OD
            uint256 operatorEthBalance = operatorDelegators[i].getStakedETHBalance();

            // Save it to the array for the OD
            operatorValues[operatorValues.length - 1] = operatorEthBalance;

            // Add it to the total TVL for this OD
            operatorTVL += operatorEthBalance;

            // Add it to the total TVL for the protocol
            totalTVL += operatorTVL;

            // Save the TVL for this OD
            operatorDelegatorTVLs[i] = operatorTVL;

            // Set withdrawQueueTokenBalanceRecorded flag to true
            withdrawQueueTokenBalanceRecorded = true;

            unchecked {
                ++i;
            }
        }

        // Get the value of native ETH held in the deposit queue and add it to the total TVL
        totalTVL += address(depositQueue).balance;

        // Add native ETH help in withdraw Queue and totalWithdrawalQueueValue to totalTVL
        totalTVL += (address(withdrawQueue).balance + totalWithdrawalQueueValue);

        return (operatorDelegatorTokenTVLs, operatorDelegatorTVLs, totalTVL);
    }

As you can see, totalTVL is determined by factors such as the balance of depositQueue, withdrawQueue, etc. These values can change while the user's deposit and withdraw calls are in the mempool.

They may not receive the expected amount of tokens that they should receive when they called these functions.

Tools Used

Manual Review.

Recommended Mitigation Steps

Add slippage protection to user deposit and withdraw functions.

Assessed type

Context

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Deposits/DepositQueue.sol#L163

Vulnerability details

Impact

In the contest details, the protocol mentions the following regarding Protocol Fees:

"Native ETH earned from outside EigenLayer (such as Execution Layer Rewards or MEV) will be sent to the DepositQueue receive() function. The protocol will then forward a configured fee percentage to an external address."

However, Execution Layer Rewards are not transferred, but rather credited. This means that the DepositQueue::receive() function will not trigger when these rewards are earned, and rewards will be lost.

Proof of Concept

DepositQueue::receive https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Deposits/DepositQueue.sol#L163

    /// @dev Handle ETH sent to this contract from outside the protocol - e.g. rewards
    /// ETH will be stored here until used for a validator deposit
    /// This should receive ETH from scenarios like Execution Layer Rewards and MEV from native staking
    /// Users should NOT send ETH directly to this contract unless they want to donate to existing ezETH holders
    /// Checks the ETH withdraw Queue and fills up if required
    receive() external payable nonReentrant {
        uint256 feeAmount = 0;
        // Take protocol cut of rewards if enabled
        if (feeAddress != address(0x0) && feeBasisPoints > 0) {
            feeAmount = (msg.value * feeBasisPoints) / 10000;
            (bool success, ) = feeAddress.call{ value: feeAmount }("");
            if (!success) revert TransferFailed();

            emit ProtocolFeesPaid(IERC20(address(0x0)), feeAmount, feeAddress);
        }
        // update remaining rewards
        uint256 remainingRewards = msg.value - feeAmount;
        // Check and fill ETH withdraw buffer if required
        _checkAndFillETHWithdrawBuffer(remainingRewards);

        // Add to the total earned
        totalEarned[address(0x0)] = totalEarned[address(0x0)] + remainingRewards;

        // Emit the rewards event
        emit RewardsDeposited(IERC20(address(0x0)), remainingRewards);
    }

As we can observe from the function and dev comments, it is expected that this function will trigger when Execution Layer rewards are received.

However, that is not the case. Execution Layer Rewards are credited rather than transferred:

"Under proof of stake, the block reward is credited to the validator's beacon chain balance, and the transaction fees are credited to the fee_recipient Ethereum address".

Source: https://eth2book.info/capella/annotated-spec/#:~:text=fee_recipient%20is%20the,fee_recipient%20Ethereum%20address

The receive() function will not trigger and rewards will be lost.

Tools Used

Manual Review.

Recommended Mitigation Steps

Add a function to the DepositQueue contract to distribute Execution Layer rewards from the contract.

Assessed type

ETH-Transfer