thebrittfactor commented 4 months ago

Lines of code

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

Vulnerability details

Impact

lose of funds for users who deposit into protocol and holding ezETH if protocol remove collateral tokens and operators while collateral tokens is staking at eigen layer and operators having staked values .

Proof of Concept

   ```function removeOperatorDelegator(
    IOperatorDelegator _operatorDelegatorToRemove
) external onlyRestakeManagerAdmin {
    // Remove it from the list
    uint256 odLength = operatorDelegators.length;
    //*@audit-issue -------->>> don't remove it when O.D have stake values 
    for (uint256 i = 0; i < odLength; ) {
        if (address(operatorDelegators[i]) == address(_operatorDelegatorToRemove)) {
            // Clear the allocation
            operatorDelegatorAllocations[_operatorDelegatorToRemove] = 0;
            emit OperatorDelegatorAllocationUpdated(_operatorDelegatorToRemove, 0);

            // Remove from list
            operatorDelegators[i] = operatorDelegators[operatorDelegators.length - 1];
            operatorDelegators.pop();
            emit OperatorDelegatorRemoved(_operatorDelegatorToRemove);
            return;
        }
        unchecked {
            ++i;
        }
    }

    // If the item was not found, throw an error
    revert NotFound();
}```

   function removeCollateralToken(
    IERC20 _collateralTokenToRemove
) external onlyRestakeManagerAdmin {
    // Remove it from the list
    //*@audit-issue --------->>> don't remove it if collateral tokens are in staked value 
    uint256 tokenLength = collateralTokens.length;
    for (uint256 i = 0; i < tokenLength; ) {
        if (address(collateralTokens[i]) == address(_collateralTokenToRemove)) {
            //*@audit-info ------>>> check that after poping last index , previous last index went to where?
            collateralTokens[i] = collateralTokens[collateralTokens.length - 1];
            collateralTokens.pop();
            emit CollateralTokenRemoved(_collateralTokenToRemove);
            return;
        }
        unchecked {
            ++i;
        }
    }

    // If the item was not found, throw an error
    revert NotFound();
}

Both remove functions for collateral tokens and operators have no check for that collateral tokens which gonna removed has staked values by each operators at eigen layer and tokens in the withdrawal queue or NOT . IF removed collateral tokens have values at protocol, it gonna make negative impact at calculating TVL. When calculating TVL , each collateral tokens value is calculated with balance of each collateral tokens in protocol multiplied by it's price.

    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;
        //*@audit-info ------>>> collateral token is removed while being on staked at eigen layer 
        //*@audit-info ------>>> while calculating total tvl , there will be missing value for staked collateral 
        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(
                    //*@audit-issue ----->>> it should be j index
                    collateralTokens[i],
                    collateralTokens[j].balanceOf(withdrawQueue)
                );
            }

            unchecked {
                ++j;
            }
        }

total value locked will be decreased due to remove collateral while having staked values . And there 's no burning the ezETH . This means value of ezETH is decreased . Due to ezETH value is decreased , user will lose funds while holding ezETH.Also Removing the operators while having stake values in the protocol will make above scenario. In the worst scenario , due to remove those collateral and operators , TVL is decreased and make the price of ezETH to below 1 ether .And then protocol will not work anymore due to check that price has to be above 1 ether in price oracle .

Tools Used

manual view

Recommended Mitigation Steps

before removing the colalteral tokens and operators , pls check that they have stake values in protocol

Assessed type

Context

thebrittfactor commented 4 months ago

For transparency, the judge has requested that issue #461 be duplicated, as it contains two issues they deemed should be judged separately.

c4-judge commented 4 months ago

alcueca marked the issue as duplicate of #5