NodeDelegator's stETH balance may exceed EigenLayer's maxPerDeposit / maxTotalDeposit, resulting in soft DoS

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/NodeDelegator.sol#L51-L68

Vulnerability details

Impact

EigenLayer has two variables that limit the incoming deposits: maxTotalDeposits and maxPerDeposit. If current deposit would exceed at least one of these two variables, that is if:

1. strategysBalanceBeforeDeposit + depositAmount > maxTotalDeposit
or
2. depositAmount > maxPerDeposit,

the deposit is reverted.

Kelp mitigates this issue by setting depositLimitByAsset for each LST to 100k:

LRTConfig.sol#L61-L63

        _addNewSupportedAsset(stETH, 100_000 ether);
        _addNewSupportedAsset(rETH, 100_000 ether);
        _addNewSupportedAsset(cbETH, 100_000 ether);

However, it does not take into account that stETH is rebasing, and the balance of NodeDelegator may exceed the 100k limit while EL is still paused.

Then, NodeDelegator is able to deposit only its whole balance, which would be reverted by EL due to one of the two previously mentioned checks:

NodeDelegator.sol#L63-L67

        uint256 balance = token.balanceOf(address(this));
        emit AssetDepositIntoStrategy(asset, strategy, balance);
        IEigenStrategyManager(eigenlayerStrategyManagerAddress).depositIntoStrategy(IStrategy(strategy), token, balance);

By the time Kelp is able to mitigate the issue (for example, by moving a part of the funds back to LRTDepositPool via transferBackToLRTDepositPool), and given that EL deposits may be open only for a few hours, users' stETH deposited into Kelp may not be deposited into EL and will not be earning EL yield for a few weeks, until EL opens deposits again.

Proof of Concept

1. Kelp accumulates 100_000e18 of stETH while EL deposits are paused.
2. Few days pass. stETH balance of the stETH NodeDelegator grows due to rebasing and is now 100_100e18.
3. EL sets maxTotalDeposit = 200_000e18, maxPerDeposit remains at 100_000e18.
4. Manager calls depositAssetIntoStrategy:

    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);
    }

depositIntoStrategy tries to deposit the whole balance of the contract. The transaction fails, because NodeDelegator's balance of stETH (100_100e18) is above maxPerDeposit, and because the deposit would exceed maxTotalDeposits as well.

Revert due to EigenLayer deposits from the outside of Kelp

There will also be EL depositors who are faster than LRTManager. If, for example, by the time LRTManager is able to deposit, there's already 40k of successful deposits, depositAssetIntoStrategy will fail (if NodeOperator's balance is above 60k). Because the time is of the essence, it is important to make sure that the deposit will be successful regardless of how much funds NodeOperator holds.

Recommended Mitigation Steps

    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 maxPerDeposit = IEigenStrategyManager(eigenlayerStrategyManagerAddress).maxPerDeposit();
+       uint256 maxDeposit = IStrategy(strategy).maxTotalDeposits() - token.balanceOf(strategy);
+       uint256 limit = maxPerDeposit < maxDeposit ? maxPerDeposit : maxDeposit;
        uint256 balance = token.balanceOf(address(this));
+       uint256 amount = balance < limit ? balance : limit;
-       emit AssetDepositIntoStrategy(asset, strategy, balance);
+       emit AssetDepositIntoStrategy(asset, strategy, amount);
        IEigenStrategyManager(eigenlayerStrategyManagerAddress)
-           .depositIntoStrategy(IStrategy(strategy), token, balance);
+           .depositIntoStrategy(IStrategy(strategy), token, amount);
    }

The new functions used in the mitigation should be added respectively to their IEigenStrategyManager and IStrategy interfaces.

Assessed type

Math

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #471

[c4-judge]

fatherGoose1 marked the issue as unsatisfactory: Invalid