function depositAssetIntoStrategy and addNodeDelegatorContractToQueue does not check if nodedelegator is frozen

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/c5fdc2e62c5e1d78769f44d6e34a6fb9e40c00f0/src/NodeDelegator.sol#L48-L68 https://github.com/code-423n4/2023-11-kelp/blob/c5fdc2e62c5e1d78769f44d6e34a6fb9e40c00f0/src/LRTDepositPool.sol#L159-L176 https://github.com/code-423n4/2023-11-kelp/blob/c5fdc2e62c5e1d78769f44d6e34a6fb9e40c00f0/src/LRTDepositPool.sol#L183-L197

Vulnerability details

Impact

function depositAssetIntoStrategy does not check if nodedelegator is frozen potentially causing funds to be lost in some situations

Proof of Concept

The function depositAssetIntoStrategy calls the function depositIntoStrategy from Eigen Layer which allows a node delegator to stake funds into a strategy.

function depositAssetIntoStrategy(address asset)
        external
        override
        whenNotPaused
        nonReentrant
        onlySupportedAsset(asset)
        onlyLRTManager
    {
        address strategy = lrtConfig.assetStrategy(asset);
        IERC20 token = IERC20(asset);
        address eigenlayerStrategyManagerAddress = lrtConfig.getContract(LRTConstants.EIGEN_STRATEGY_MANAGER);

        uint256 balance = token.balanceOf(address(this));

        emit AssetDepositIntoStrategy(asset, strategy, balance);

        IEigenStrategyManager(eigenlayerStrategyManagerAddress).depositIntoStrategy(IStrategy(strategy), token, balance);
    }

This Node delegator is chosen from an array created in LRTDepositPool.sol (function addNodeDelegatorContractToQueue.sol)

        uint256 length = nodeDelegatorContracts.length;
        if (nodeDelegatorQueue.length + length > maxNodeDelegatorCount) {
            revert MaximumNodeDelegatorCountReached();
        }

        for (uint256 i; i < length;) {
            UtilLib.checkNonZeroAddress(nodeDelegatorContracts[i]);
            nodeDelegatorQueue.push(nodeDelegatorContracts[i]);
            emit NodeDelegatorAddedinQueue(nodeDelegatorContracts[i]);
            unchecked {
                ++i;
            }
        }
    }

However there is a missing check in both the depositAssetIntoStrategy function and the function addNodeDelegatorContractToQueue on whether the node delegator is frozen.

https://github.com/code-423n4/2023-04-eigenlayer/blob/5e4872358cd2bda1936c29f460ece2308af4def6/src/contracts/core/StrategyManager.sol#L220-L228

In Eigen Layer, this check is made to prevent an address of an operator who has been slashed or from a staker who is actively delegated to a slashed operator.

Without this check, there can be unexpected reverts and in some cases, a freezing of funds if the node delegator is frozen after depositing funds. This is because Eigen Layer has an iffrozen check on both deposit and withdrawal functions

https://github.com/code-423n4/2023-04-eigenlayer/blob/5e4872358cd2bda1936c29f460ece2308af4def6/src/contracts/core/StrategyManager.sol#L223

https://github.com/code-423n4/2023-04-eigenlayer/blob/5e4872358cd2bda1936c29f460ece2308af4def6/src/contracts/core/StrategyManager.sol#L338

Tools Used

Manual Review

Recommended Mitigation Steps

Before adding a node delegator or making a deposit to the node delegator, check whether the node delegator is not frozen from Eigen Layer

Assessed type

Context

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as primary issue

[raymondfam]

The fund can be retrieved via transferBackToLRTDepositPool() after all since Eigen Layer stops accepting deposits from the node delegator. Additionally, this is a restricted call by the manager.

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-judge]

fatherGoose1 marked the issue as unsatisfactory: Invalid