Renzo - lanrebayode77's results

Lines of code

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

Vulnerability details

Impact

If there are more Operator Delegators than collateral tokens, there will be an out of bound error due to how calculateTVLs() was implemented and user exposed function like deposit() will revert.

In RestakeManager.calculateTVLs(), the function loops through the entire Operator delegators to get each operator's tvl.

The problem here is that, when the function tries to increment totalWithdrawalQueueValue by calling Renzo oracle to lookup value of the token in withdrawQueue, it looks at the wrong index to pass the token instance.

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

From the code snippet above, it's observed that the index of the operator delegators was used instead of tokens index.

Other functions that will be affected include; depositTokenRewardsFromProtocol() and depositEth().

Proof of Concept

If for instance, there are 10 operators and just 3 EERC20 tokens, looking at index 3 and above for a collateral token instance will lead to out of bound error and function will revert.

The likelihood of this happening is high and the severity is high as well, since deposit() will remain DOSed.

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(); //@audit-issue will cause revert

At the moment only stEth and wbEth seems to be the only accepted tokens, with three or more operator validators, deposit() will remain DOSed.

Tools Used

Manual review.

Recommended Mitigation Steps

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

Assessed type

DoS

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/RestakeManager.sol#L547-L549 https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/RestakeManager.sol#L561-L562 https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/Delegation/OperatorDelegator.sol#L147-L148

Vulnerability details

Impact

RestakeManager.deposit() will revert whenever amount being deposited is lower or equal to buffer deficit.

Proof of Concept

1. If _amount <= bufferToFill, _amount = 0 after subtraction.

  if (bufferToFill > 0) {
            bufferToFill = (_amount <= bufferToFill) ? _amount : bufferToFill;
            // update amount to send to the operator Delegator
         @>  _amount -= bufferToFill; //@audit-issue if _amount <= bufferToFill, _amount = 0 after subtraction,  deposit in OD will revert

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

            // fill Withdraw Buffer via depositQueue
            depositQueue.fillERC20withdrawBuffer(address(_collateralToken), bufferToFill);
        }

2. OD.deposit() is called without zero check!

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

        // Call deposit on the operator delegator
        operatorDelegator.deposit(_collateralToken, _amount); //@audit-issue revert with 0 amoount!

3. deposit() reverts in OD.

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

Tools Used

Manual review

Recommended Mitigation Steps

Add a zero check in RestakeManager.deposit()

 // Approve the tokens to the operator delegator

        _collateralToken.safeApprove(address(operatorDelegator), _amount); 

     -- // Call deposit on the operator delegator
     ++ // Call deposit on the operator delegator if _amount > 0
    if(_amount > 0) {
        operatorDelegator.deposit(_collateralToken, _amount); //@audit-ok 
  }
        

Assessed type

DoS

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/Deposits/DepositQueue.sol#L252-L277 https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/RestakeManager.sol#L286-L287 https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/RestakeManager.sol#L351-L355

Vulnerability details

Impact

calculateTVLs() fails to account for erc20 tokens in DepositQueue contract leading to incorrect TVL calculation.

This will lead to an incorrect check in deposit() and TVL limit will not actually hold, since a lower value is being returned than the actual value.

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

Proof of Concept

Total value of each erc20 tokens in withdrawQueue contract was included in tvl calculation

  // record token value of withdraw queue
                if (!withdrawQueueTokenBalanceRecorded) {
                    totalWithdrawalQueueValue += renzoOracle.lookupTokenValue(
                        collateralTokens[i], //@audit should be j not i; 
                        collateralTokens[j].balanceOf(withdrawQueue) //@audit should include depositQueue balance
                    );
                }

value of ETH in withdrawQueue was also included in tvl total

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

Now, tvl total included ETH in depositQueue

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

But failed to include depositQueue erc20 tokens value to the total TVL. However, it is possible that there are erc20 tokens present in the depositQueue waiting to be swept by an admin.

    /// @dev Sweeps any accumulated ERC20 tokens in this contract to the RestakeManager
    /// Only callable by a permissioned account
    function sweepERC20(IERC20 token) external onlyERC20RewardsAdmin {
        uint256 balance = IERC20(token).balanceOf(address(this));
        if (balance > 0) {
            uint256 feeAmount = 0;

            // Sweep fees if configured
            if (feeAddress != address(0x0) && feeBasisPoints > 0) {
                feeAmount = (balance * feeBasisPoints) / 10000;
                IERC20(token).safeTransfer(feeAddress, feeAmount);

                emit ProtocolFeesPaid(token, feeAmount, feeAddress);
            }

            // Approve and deposit the rewards
            token.approve(address(restakeManager), balance - feeAmount);
            restakeManager.depositTokenRewardsFromProtocol(token, balance - feeAmount);

            // Add to the total earned
            totalEarned[address(token)] = totalEarned[address(token)] + balance - feeAmount;

            // Emit the rewards event
            emit RewardsDeposited(IERC20(address(token)), balance - feeAmount);
        }
    }

Tools Used

Manual review

Recommended Mitigation Steps

Include erc20 tokens value present in depositQueue contract the way it was done for withdrawQueue

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

Assessed type

Context