Renzo - DanielArmstrong's results

Lines of code

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

Vulnerability details

Impact

The total TVL is miscalculated. The issue will cause many unexpected errors.

Proof of Concept

RestakeManager.sol#calculateTVLs function is the following.

    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(
318:                    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);
    }

totalWithdrawalQueueValue means the total sum of values of collaterals in the withdrawQueue. But L318 multiplies the value of collateralTokens[i] with collateralTokens[j].balanceOf(withdrawQueue). Since the value of collateralTokens[i] differs from the value of collateralTokens[j], the totalWithdrawalQueueValue is miscalculated.

Tools Used

Manual Review

Recommended Mitigation Steps

Modify RestakeManager.sol#calculateTVLs function as follows.

    function calculateTVLs() public view returns (uint256[][] memory, uint256[] memory, uint256) {
        ......
                // 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/RestakeManager.sol#L592 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Withdraw/WithdrawQueue.sol#L206

Vulnerability details

Impact

When a user call RestakeManager.sol#depositETH function, slippage can arise and the user will not receive the expected amount of ezETH. The same problem also exists in RestakeManager.sol#deposit and WithdrawQueue.sol#withdraw functions.

Proof of Concept

RestakeManager.sol#depositETH function is the following.

    function depositETH(uint256 _referralId) public payable nonReentrant notPaused {
        // Get the total TVL
594:    (, , uint256 totalTVL) = calculateTVLs();

        // Enforce TVL limit if set
        if (maxDepositTVL != 0 && totalTVL + msg.value > maxDepositTVL) {
            revert MaxTVLReached();
        }

        // Deposit the remaining ETH into the DepositQueue
        depositQueue.depositETHFromProtocol{ value: msg.value }();

        // Calculate how much ezETH to mint
605:    uint256 ezETHToMint = renzoOracle.calculateMintAmount(
            totalTVL,
            msg.value,
            ezETH.totalSupply()
        );

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

        // Emit the deposit event
        emit Deposit(msg.sender, IERC20(address(0x0)), msg.value, ezETHToMint, _referralId);
    }

As can be seen, the above function has no parameter for slippage control. Therefore, if the prices of collaterals change while the function call stays in the mempool, then totalTVL will change in L594 and thus ezETHToMint will change in L605 too. That is, the slippage can arise and cause damage to the caller.

The same problem also exists in RestakeManager.sol#deposit and WithdrawQueue.sol#withdraw functions.

Tools Used

Manual Review

Recommended Mitigation Steps

Insert a parameter for slippage control into RestakeManager.sol#depositETH function as the following.

--  function depositETH(uint256 _referralId) public payable nonReentrant notPaused {
++  function depositETH(uint256 _referralId, uint256 minAmount) public payable nonReentrant notPaused {
        // Get the total TVL
        (, , uint256 totalTVL) = calculateTVLs();

        // Enforce TVL limit if set
        if (maxDepositTVL != 0 && totalTVL + msg.value > maxDepositTVL) {
            revert MaxTVLReached();
        }

        // Deposit the remaining ETH into the DepositQueue
        depositQueue.depositETHFromProtocol{ value: msg.value }();

        // Calculate how much ezETH to mint
        uint256 ezETHToMint = renzoOracle.calculateMintAmount(
            totalTVL,
            msg.value,
            ezETH.totalSupply()
        );
++      if (ezETHToMint < minAmount) {
++          revert;
++      }

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

        // Emit the deposit event
        emit Deposit(msg.sender, IERC20(address(0x0)), msg.value, ezETHToMint, _referralId);
    }
    }

Modify RestakeManager.sol#deposit and WithdrawQueue.sol#withdraw functions also in the same way.

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

Depsits of amount which is smaller than bufferToFill will be reverted.

Proof of Concept

RestakeManager.sol#deposit function is the following.

    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
549:        _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
562:    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);
    }

When bufferToFill > amount, _amount will become zero in L549. Therefore, _amount == 0 holds true in L562.

In the meantime, the OperatorDelegator.sol#deposit function is the following.

    function deposit(
        IERC20 token,
        uint256 tokenAmount
    ) external nonReentrant onlyRestakeManager returns (uint256 shares) {
147:    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);
    }

As we can see, when tokenAmount == 0 in L147, the above function will be reverted.

Tools Used

Manual Review

Recommended Mitigation Steps

Modify RestakeManager.sol#deposit function as follows.

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

++      if (_amount > 0) {
        // 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);
    }

Assessed type

Math