code-423n4 / 2024-04-renzo-findings

6 stars 5 forks source link

Incorrect TVL calculation in `DepositQueue` because of ignoring the ERC20 balance #384

Open howlbot-integration[bot] opened 2 months ago

howlbot-integration[bot] commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/1c7cc4e632564349b204b4b5e5f494c9b0bc631d/contracts/RestakeManager.sol#L352

Vulnerability details

Impact

The TVL (Total Value Locked) calculation in the protocol does not consider any accumulated ERC20 tokens in the DepositQueue contract. This leads to an inaccurate TVL calculation and consequently affects the valuation of ezETH.

Proof of Concept

  1. TVL Calculation: When calculating the TVL, only the ETH balance of the DepositQueue contract is considered. Any ERC20 tokens accumulated in this contract are not factored into the TVL calculation.

    
    function calculateTVLs() public view returns (uint256[][] memory, uint256[] memory, uint256) {
    //....
    
    // Get the value of native ETH held in the deposit queue and add it to the total TVL
    totalTVL += address(depositQueue).balance;
    
    //....

}

https://github.com/code-423n4/2024-04-renzo/blob/1c7cc4e632564349b204b4b5e5f494c9b0bc631d/contracts/RestakeManager.sol#L352

2. **ERC20 Token Handling**: If the admin calls the `sweepERC20` function, the accumulated ERC20 tokens in `DepositQueue` are transferred to the `RestakeManager` and deposited into a strategy. 
```solidity
function sweepERC20(IERC20 token) external onlyERC20RewardsAdmin {
        //...

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

        //...
    }

https://github.com/code-423n4/2024-04-renzo/blob/1c7cc4e632564349b204b4b5e5f494c9b0bc631d/contracts/Deposits/DepositQueue.sol#L269

Tools Used

Recommended Mitigation Steps

Two recommendations are proposed:

  1. Include ERC20 Balance in TVL: Modify the TVL calculation to include the ERC20 balance of DepositQueue after deducting any fee amounts.

    function calculateTVLs() public view returns (uint256[][] memory, uint256[] memory, uint256) {
        //...
            for (uint256 j = 0; j < tokenLength; ) {
                //...
                uint balance = collateralTokens[j].balanceOf(depositQueue);
                uint feeAmount;
                if (!depositQueueTokenBalanceRecorded && balance > 0) {
                    if (feeAddress != address(0x0) && feeBasisPoints > 0) {
                        feeAmount = (balance * feeBasisPoints) / 10000;
                    }
                    totalDepositQueueValue += renzoOracle.lookupTokenValue(
                        collateralTokens[j],
                        balance - feeAmount
                    );
                }
            }
    
        //...
    
        totalTVL += address(depositQueue).balance + totalDepositQueueValue;
    }
  2. Automate ERC20 Sweep: Whenever TVL is calculated, automatically check the ERC20 balance of DepositQueue. If nonzero, trigger the sweepERC20 function to ensure accurate TVL calculation.

    function calculateTVLs() public view returns (uint256[][] memory, uint256[] memory, uint256) {
        //...
            for (uint256 j = 0; j < tokenLength; ) {
                //...
                uint balance = collateralTokens[j].balanceOf(depositQueue);
                if (!depositQueueTokenBalanceRecorded && balance > 0) {
                    depositQueue.sweepERC20(collateralTokens[j]);
                }
            }
    
        //...
    }

Assessed type

Context

c4-judge commented 2 months ago

alcueca marked the issue as not a duplicate

jatinj615 commented 2 months ago

similar to #379

c4-judge commented 2 months ago

alcueca changed the severity to QA (Quality Assurance)

c4-judge commented 2 months ago

alcueca marked the issue as grade-a

c4-judge commented 2 months ago

alcueca marked the issue as grade-b