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

[howlbot-integration[bot]]

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

[C4-Staff]

CloudEllie marked the issue as duplicate of #381

[c4-judge]

alcueca marked the issue as not a duplicate

[c4-judge]

alcueca marked the issue as duplicate of #381

[c4-judge]

alcueca marked the issue as satisfactory

[c4-judge]

alcueca changed the severity to QA (Quality Assurance)