BEACON CHAIN VALIDATOR COULD SELF RESCUE WHEN OPERATOR IS FROZEN

[code423n4]

Lines of code

https://github.com/code-423n4/2023-04-eigenlayer/blob/main/src/contracts/core/StrategyManager.sol#L195-L198 https://github.com/code-423n4/2023-04-eigenlayer/blob/main/src/contracts/core/StrategyManager.sol#L699-L705 https://github.com/code-423n4/2023-04-eigenlayer/blob/main/src/contracts/core/StrategyManager.sol#L807-L814

Vulnerability details

Impact

The modifier onlyNotFrozen() is intuitive such that the staker will be frozen when the delegated operator is frozen. However, not utilizing it in recordOvercommittedBeaconChainETH() and undelegate() could allow the Beacon Chain validator to undelegate from the frozen operator and escape the slash.

Proof of Concept

Let's assume the validator has only beaconChainETHStrategy in his/her stakerStrategy. It happens that the operator delegated is doing something not right and now frozen. The validator, after realizing this, purposely makes himself/herself over committed resulting in all shares removed from the enshrined beacon chain ETH strategy:

https://github.com/code-423n4/2023-04-eigenlayer/blob/main/src/contracts/core/StrategyManager.sol#L195-L198

        // removes shares for the enshrined beacon chain ETH strategy
        if (amount != 0) {
            _removeShares(overcommittedPodOwner, beaconChainETHStrategyIndex, beaconChainETHStrategy, amount);            
        }

Since amount is REQUIRED_BALANCE_WEI, the same amount virtually deposited for new ETH validator, assuming that no rewards have been given, no existing shares is left and hence the strategy is removed from the depositor's dynamic array of strategies.

https://github.com/code-423n4/2023-04-eigenlayer/blob/main/src/contracts/core/StrategyManager.sol#L699-L705

        // if no existing shares, remove the strategy from the depositor's dynamic array of strategies
        if (userShares == 0) {
            _removeStrategyFromStakerStrategyList(depositor, strategyIndex, strategy);

            // return true in the event that the strategy was removed from stakerStrategyList[depositor]
            return true;
        }

The validator can now undelegate from the operator:

https://github.com/code-423n4/2023-04-eigenlayer/blob/main/src/contracts/core/StrategyManager.sol#L807-L814

    /**
     * @notice If the `depositor` has no existing shares, then they can `undelegate` themselves.
     * This allows people a "hard reset" in their relationship with EigenLayer after withdrawing all of their stake.
     */
    function _undelegate(address depositor) internal {
        require(stakerStrategyList[depositor].length == 0, "StrategyManager._undelegate: depositor has active deposits");
        delegation.undelegate(depositor);
    }

The validator is now unfrozen and will escape the slash. On top of that, he/she is free to make a full withdrawal undeterred.

Recommended Mitigation Steps

It is recommended imposing onlyNotFrozen on recordOvercommittedBeaconChainETH() and undelegate() to circumvent the leaked path.

Assessed type

Timing

[c4-pre-sort]

0xSorryNotSorry marked the issue as duplicate of #210

[c4-judge]

GalloDaSballo marked the issue as duplicate of #210

[c4-judge]

GalloDaSballo marked the issue as not a duplicate

[c4-judge]

GalloDaSballo marked the issue as duplicate of #408

[c4-judge]

GalloDaSballo marked the issue as satisfactory