c4-bot-5 commented 6 months ago

Lines of code

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

Vulnerability details

Impact

Depositing could be reverted due to improper choosing of the operator delegator, even though there may be another appropriate one available.

Proof of Concept

Some strategies of EigenLayer have deposit limitation variables maxPerDeposit and maxTotalDeposits. So, if the depositing amount exceeds maxPerDeposit or causes an overflow of the strategy's total deposit amount, then the transaction will be reverted.

However, the chooseOperatorDelegatorForDeposit() function does not consider the amount to be deposited. So, if the strategy of the operator delegator chosen by the chooseOperatorDelegatorForDeposit() function cannot accommodate the given deposit amount, the transaction will be reverted. In this case, it would be better for the function to choose another, more suitable operator delegator.

Especially in a scenario where there is no delegator that doesn't exceed it's allocation limit, the function will only choose the first delegator. This increases the likelihood of the above problem occurring, as all deposits will be concentrated on the first delegator.

    function chooseOperatorDelegatorForDeposit(
        uint256[] memory tvls,
        uint256 totalTVL
    ) public view returns (IOperatorDelegator) {
        // Ensure OperatorDelegator list is not empty
        if (operatorDelegators.length == 0) revert NotFound();

        // If there is only one operator delegator, return it
        if (operatorDelegators.length == 1) {
            return operatorDelegators[0];
        }

        // Otherwise, find the operator delegator with TVL below the threshold
        uint256 tvlLength = tvls.length;
        for (uint256 i = 0; i < tvlLength; ) {
            if (
                tvls[i] <
                (operatorDelegatorAllocations[operatorDelegators[i]] * totalTVL) /
                    BASIS_POINTS /
                    BASIS_POINTS
            ) {
                return operatorDelegators[i];
            }

            unchecked {
                ++i;
            }
        }

        // Default to the first operator delegator
        return operatorDelegators[0];
    }

Tools Used

Manual review

Recommended Mitigation Steps

The mechanism for choosing the operator delegator for a deposit should be improved to account for the deposit limitation variables of the strategies.

Assessed type

Context

jatinj615 commented 6 months ago

The collateralTokenTVL limits are enforced on deposits. Also the max Cap on LST deposits have been removed by EigenLayer.

C4-Staff commented 6 months ago

CloudEllie marked the issue as primary issue

alcueca commented 6 months ago

A review of the current parameters for EigenLayer strategies shows indeed that the caps have been removed.

Downgrading to QA, as it is valid input for future iterations of the RestakeManager.