`withdrawBeforeRestaking()` can't be called in case of second stake with different validator and can cause ether balance to stuck

[code423n4]

Lines of code

https://github.com/code-423n4/2023-04-eigenlayer/blob/main/src/contracts/pods/EigenPod.sol#L454-L457 https://github.com/code-423n4/2023-04-eigenlayer/blob/main/src/contracts/pods/EigenPod.sol#L114-L117

Vulnerability details

Impact

User can deposit beacon chain ETH trough the EigenPodManager and call stake, EigenPod created if user do not have one, then stake called inside EigenPod that will trigger "ETH2 Deposit Contract" with passing the required eth and parameters including _podWithdrawalCredentials. Suppose user successfully finished the Beacon Chain ETH deposit process by calling EigenPod.verifyWithdrawalCredentialsAndBalance after the appropriate condition is fulfilled.

The second time user go trough deposit beacon chain ETH with different validator in the same EigenPod, but this time not finishing it with EigenPod.verifyWithdrawalCredentialsAndBalance and not yet make the new validator status active inside the EigenPod, instead he want to withdraw it with triggering a full withdrawal from the Beacon Chain. The balance will be send to EigenPod but user can't call withdrawBeforeRestaking and make the eth will stuck inside EigenPod.

Proof of Concept

Consider following scenario :

1. User deposit beacon chain ETH, call EigenPodManager.stake.

https://github.com/code-423n4/2023-04-eigenlayer/blob/main/src/contracts/pods/EigenPodManager.sol#L112-L119

    function stake(bytes calldata pubkey, bytes calldata signature, bytes32 depositDataRoot) external payable {
        IEigenPod pod = ownerToPod[msg.sender];
        if(address(pod) == address(0)) {
            //deploy a pod if the sender doesn't have one already
            pod = _deployPod();
        }
        pod.stake{value: msg.value}(pubkey, signature, depositDataRoot);
    }

2. It will create EigenPod if don't have one, and will call pod.stake with providing the msg.value and the required parameters, EigenPod's stake will check the msg.value, and call ethPOS.deposit with also passing the required parameters, including the pod withdrawal credentials :

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

    function stake(bytes calldata pubkey, bytes calldata signature, bytes32 depositDataRoot) external payable onlyEigenPodManager {
        // stake on ethpos
        require(msg.value == 32 ether, "EigenPod.stake: must initially stake for any validator with 32 ether");
        ethPOS.deposit{value : 32 ether}(pubkey, _podWithdrawalCredentials(), signature, depositDataRoot);
        emit EigenPodStaked(pubkey);
    }

3. After the Beacon Chain state root updated, user call EigenPod.verifyWithdrawalCredentialsAndBalance to finalize the deposit, and update hasRestaked state to true.

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

    function verifyWithdrawalCredentialsAndBalance(
        uint64 oracleBlockNumber,
        uint40 validatorIndex,
        BeaconChainProofs.ValidatorFieldsAndBalanceProofs calldata proofs,
        bytes32[] calldata validatorFields
    )
        external
        onlyWhenNotPaused(PAUSED_EIGENPODS_VERIFY_CREDENTIALS)
        // check that the provided `oracleBlockNumber` is after the `mostRecentWithdrawalBlockNumber`
        proofIsForValidBlockNumber(oracleBlockNumber)
    {
        require(validatorStatus[validatorIndex] == VALIDATOR_STATUS.INACTIVE,
            "EigenPod.verifyCorrectWithdrawalCredentials: Validator must be inactive to prove withdrawal credentials");

        require(validatorFields[BeaconChainProofs.VALIDATOR_WITHDRAWAL_CREDENTIALS_INDEX] == bytes32(_podWithdrawalCredentials()),
            "EigenPod.verifyCorrectWithdrawalCredentials: Proof is not for this EigenPod");
        // deserialize the balance field from the balanceRoot
        uint64 validatorCurrentBalanceGwei = BeaconChainProofs.getBalanceFromBalanceRoot(validatorIndex, proofs.balanceRoot);

        // make sure the balance is greater than the amount restaked per validator
        require(validatorCurrentBalanceGwei >= REQUIRED_BALANCE_GWEI,
            "EigenPod.verifyCorrectWithdrawalCredentials: ETH validator's balance must be greater than or equal to the restaked balance per validator");

        // verify ETH validator proof
        bytes32 beaconStateRoot = eigenPodManager.getBeaconChainStateRoot(oracleBlockNumber);
        BeaconChainProofs.verifyValidatorFields(
            validatorIndex,
            beaconStateRoot,
            proofs.validatorFieldsProof,
            validatorFields
        );

        // verify ETH validator's current balance, which is stored in the `balances` container of the beacon state
        BeaconChainProofs.verifyValidatorBalance(
            validatorIndex,
            beaconStateRoot,
            proofs.validatorBalanceProof,
            proofs.balanceRoot
        );
        // set the status to active
        validatorStatus[validatorIndex] = VALIDATOR_STATUS.ACTIVE;

        // Sets "hasRestaked" to true if it hasn't been set yet. 
        if (!hasRestaked) {
            hasRestaked = true;
        }

        emit ValidatorRestaked(validatorIndex);

        // virtually deposit REQUIRED_BALANCE_WEI for new ETH validator
        eigenPodManager.restakeBeaconChainETH(podOwner, REQUIRED_BALANCE_WEI);
    }

4. After some time, User deposit beacon chain ETH again with new validator and not finalize it, instead trigger full withdrawal from the Beacon Chain. The balance of EigenPod should be updated with the appropriate value. But can't finish withdrawBeforeRestaking since the hasNeverRestaked modifier will revert since hasRestaked is true.

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

    function withdrawBeforeRestaking() external onlyEigenPodOwner hasNeverRestaked {
        mostRecentWithdrawalBlockNumber = uint32(block.number);
        _sendETH(podOwner, address(this).balance);
    }

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

```solidity
    modifier hasNeverRestaked {
        require(!hasRestaked, "EigenPod.hasNeverRestaked: restaking is enabled");
        _;
    }

5. The eth will stuck in EigenPod and there is no way to withdraw it.

Tools Used

Manual Review

Recommended Mitigation Steps

Consider to either prevent more than one validator deposit to EigenPod or change the hasRestaked to mapping state per validatorIndex

Assessed type

Invalid Validation

[c4-pre-sort]

0xSorryNotSorry marked the issue as primary issue

[c4-sponsor]

Sidu28 marked the issue as sponsor disputed

[Sidu28]

"There appear to be two cases here -- either: 1) The second validator was withdrawn after mostRecentWithdrawalBlockNumber, in which case the user can call verifyWithdrawalCredentialsAndBalance for the validator and proceed through the normal withdrawal flow 2) The second validator was withdrawn before mostRecentWithdrawalBlockNumber, in which case the ETH would have been credited to the EigenPod in the EigenPod and been withdrawn when withdrawBeforeRestaking was called Neither case appears to be an issue."

[GalloDaSballo]

Agree with the Sponsor that this is not an issue, ultimately the hasRestaked flag works as a one off, but the flow of funds is not blocked

Dowgrading to QA

[c4-judge]

GalloDaSballo changed the severity to QA (Quality Assurance)