code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-04-eigenlayer/blob/main/src/contracts/pods/EigenPod.sol#L292-L293 https://github.com/code-423n4/2023-04-eigenlayer/blob/main/src/contracts/core/StrategyManager.sol#L189-L198 https://github.com/code-423n4/2023-04-eigenlayer/blob/main/src/contracts/core/StrategyManager.sol#L821-L836

Vulnerability details

Impact

A Beacon Chain validator who has his/her retaked ETH slashed, and then incurs a debt due to over commitment could end up having the debt amount not withdrawable by any party.

Proof of Concept

First off, Beacon Chain reward payments will be automatically triggered every few days with each validator sweep to EigenPod.sol that has been used as the withdrawal credentials. In the event the restaked ETH is slashed, the contract owner of StrategyManager.sol will understandably make the efforts claiming the slash amount at the expense of the rewards already swept to EigenPod.sol.

Next, when the validator runs into an over commitment, a debt is going to be incurred and assigned to beaconChainETHSharesToDecrementOnWithdrawal prior to removing shares for the enshrined beacon chain ETH strategy:

https://github.com/code-423n4/2023-04-eigenlayer/blob/main/src/contracts/pods/EigenPod.sol#L292-L293

        // remove and undelegate shares in EigenLayer
        eigenPodManager.recordOvercommittedBeaconChainETH(podOwner, beaconChainETHStrategyIndex, REQUIRED_BALANCE_WEI);

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

        // if the amount exceeds the user's shares, then record it as an amount to be "paid off" when the user completes a withdrawal
        if (amount > userShares) {
            uint256 debt = amount - userShares;
            beaconChainETHSharesToDecrementOnWithdrawal[overcommittedPodOwner] += debt;
            amount -= debt;
        }
        // removes shares for the enshrined beacon chain ETH strategy
        if (amount != 0) {
            _removeShares(overcommittedPodOwner, beaconChainETHStrategyIndex, beaconChainETHStrategy, amount);            
        }

When the validator exits Beacon Chain and makes a full withdrawal, the slashed amount equivalent to the debt has already been claimed by the contract owner of StrategyManager.sol at the time when beaconChainETHSharesToDecrementOnWithdrawal[staker] was still equal to zero.

Now, as apparently evidenced from the code logic of _withdrawBeaconChainETH() below, it will decrease amount appropriately, so less is sent at the end.

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

    function _withdrawBeaconChainETH(address staker, address recipient, uint256 amount) internal {
        uint256 amountToDecrement = beaconChainETHSharesToDecrementOnWithdrawal[staker];
        if (amountToDecrement != 0) {
            if (amount > amountToDecrement) {
                beaconChainETHSharesToDecrementOnWithdrawal[staker] = 0;
                // decrease `amount` appropriately, so less is sent at the end
                amount -= amountToDecrement;
            } else {
                beaconChainETHSharesToDecrementOnWithdrawal[staker] = (amountToDecrement - amount);
                // rather than setting `amount` to 0, just return early
                return;
            }
        }
        // withdraw the beaconChainETH to the recipient
        eigenPodManager.withdrawRestakedBeaconChainETH(staker, recipient, amount);
    }

As a result, the debt amount is left stuck in EigenPod.sol. Neither can the contract owner of StrategyManager.sol claim it since there are no more shares to remove.

Recommended Mitigation Steps

It is recommended refactoring the code logic of _withdrawBeaconChainETH) such that if the debt has eralier been claimed by the contract owner of StrategyManager.sol via slashShares(). If so, beaconChainETHSharesToDecrementOnWithdrawal[staker] should first be decremented so the last line of code, eigenPodManager.withdrawRestakedBeaconChainETH(staker, recipient, amount) can directly execute.