Aymen0909 - Unable to Claim EigenLayer Withdrawals through `settleEpochFromEigenLayer`

[sherlock-admin]

Aymen0909

medium

Unable to Claim EigenLayer Withdrawals through settleEpochFromEigenLayer

Summary

If the deposit pool contains enough funds to settle an epoch after that epoch was already queued into EigenLayer withdrawal through queueCurrentEpochSettlement, the settleEpochFromEigenLayer function will be DoSed, and it will be impossible to claim the EigenLayer withdrawals for that epoch.

Vulnerability Detail

In the RioLRTCoordinator contract, when the rebalance function is called, there are two ways in which a withdrawal can proceed (after invoking _processUserWithdrawalsForCurrentEpoch under the hood):

• The deposit pool has enough funds to cover all the withdrawal, so RioLRTWithdrawalQueue.settleCurrentEpoch is called.

• Or the deposit pool can only cover a part of the withdrawal shares, and we must request a withdrawal from EigenLayer by calling RioLRTWithdrawalQueue.queueCurrentEpochSettlement. The withdrawal can later be completed (after EigenLayer 7 days delay) by calling RioLRTWithdrawalQueue.settleEpochFromEigenLayer to receive the funds from EigenLayer.

The _processUserWithdrawalsForCurrentEpoch function will handle all the withdrawal logic in the rebalancing call:

function _processUserWithdrawalsForCurrentEpoch(address asset, uint256 sharesOwed) internal {
    IRioLRTWithdrawalQueue withdrawalQueue_ = withdrawalQueue();
    (uint256 assetsSent, uint256 sharesSent) = depositPool().transferMaxAssetsForShares(
        asset,
        sharesOwed,
        address(withdrawalQueue_)
    );
    uint256 sharesRemaining = sharesOwed - sharesSent;

    // Exit early if all pending withdrawals were paid from the deposit pool.
    if (sharesRemaining == 0) {
        withdrawalQueue_.settleCurrentEpoch(asset, assetsSent, sharesSent);
        return;
    }

    address strategy = assetRegistry().getAssetStrategy(asset);
    bytes32 aggregateRoot = OperatorOperations.queueWithdrawalFromOperatorsForUserSettlement(
        operatorRegistry(),
        strategy,
        sharesRemaining
    );

    withdrawalQueue_.queueCurrentEpochSettlement(asset, assetsSent, sharesSent, aggregateRoot);
}

In this rebalance process, there is a scenario in which all the queued EigenLayer withdrawals can't be claimed at all through settleEpochFromEigenLayer and will be lost.

The following example illustrates that scenario:

• We are currently in epoch 8 (i.e., getCurrentEpoch() = 8).

• When rebalance gets called, the deposit pool hasn't enough funds to cover the full withdrawal, so the function must queue withdrawal from operators, and both RioLRTOperatorDelegator.queueWithdrawalForUserSettlement and RioLRTWithdrawalQueue.queueCurrentEpochSettlement will be invoked.

• The queueCurrentEpochSettlement will store the asset received from the deposit pool and their corresponding shares value into epochWithdrawals.assetsReceived and epochWithdrawals.shareValueOfAssetsReceived, respectively, and will decrement the amount to burn epochWithdrawals.amountToBurnAtSettlement.

• After the rebalanceDelay has passed, the rebalance function gets called again, and this time, the deposit pool can cover the full withdrawal (suppose it was filled during the rebalance delay), and so settleCurrentEpoch function will be called.

• The settleCurrentEpoch function will override the previously set epochWithdrawals.assetsReceived and epochWithdrawals.shareValueOfAssetsReceived and will burn the remaining epochWithdrawals.amountToBurnAtSettlement and will settle the epoch by setting epochWithdrawals.settled = true.

• Thus users are now able to claim their withdrawal for that epoch through claimWithdrawalsForEpoch, and we are now in epoch 9.

• The issue now is for that EigenLayer queued withdrawals that was initiated when rebalance was called the first time, the only way to claim them back is through the settleEpochFromEigenLayer function but this one will revert as the corresponding epoch (epoch 8) was already settled when settleCurrentEpoch was called in the second rebalance call.

function settleEpochFromEigenLayer(
    address asset,
    uint256 epoch,
    IDelegationManager.Withdrawal[] calldata queuedWithdrawals,
    uint256[] calldata middlewareTimesIndexes
) external {
    EpochWithdrawals storage epochWithdrawals = _getEpochWithdrawals(asset, epoch);
    if (epochWithdrawals.sharesOwed == 0) revert NO_SHARES_OWED_IN_EPOCH();
    // @audit will revert as epoch is already settled
    if (epochWithdrawals.settled) revert EPOCH_ALREADY_SETTLED();
    if (epochWithdrawals.aggregateRoot == bytes32(0)) revert WITHDRAWALS_NOT_QUEUED_FOR_EPOCH();
    ...
}

• So the settleEpochFromEigenLayer is basically DoSed for the epoch 8 as it was settled, and because the only entity that can claim those queued withdrawals is RioLRTWithdrawalQueue contract as it was set when RioLRTOperatorDelegator.queueWithdrawalForUserSettlement was called, those withdrawals can never be claimed again:

function queueWithdrawalForUserSettlement(address strategy, uint256 shares) external onlyCoordinator returns (bytes32 root) {
    if (strategy == BEACON_CHAIN_STRATEGY) {
        _increaseETHQueuedForUserSettlement(shares);
    }
    root = _queueWithdrawal(strategy, shares, address(withdrawalQueue()));
}

• This will result in a loss of funds for the protocol and the users, add to that because settleEpochFromEigenLayer wasn't called neither decreaseSharesHeldForAsset or decreaseETHQueuedForUserSettlement were called which means that the protocol accounting will be wrong after that.

Impact

If the deposit pool contains enough funds to settle an epoch after that epoch was already queued into EigenLayer withdrawal through queueCurrentEpochSettlement, the settleEpochFromEigenLayer function will be DoSed, and it will be impossible to claim the EigenLayer withdrawals, resulting in a loss of funds for the protocol and the users, and making all the shares accounting logic wrong.

Code Snippet

https://github.com/sherlock-audit/2024-02-rio-network-core-protocol/blob/main/rio-sherlock-audit/contracts/restaking/RioLRTCoordinator.sol#L245-L267

https://github.com/sherlock-audit/2024-02-rio-network-core-protocol/blob/main/rio-sherlock-audit/contracts/restaking/RioLRTWithdrawalQueue.sol#L216-L271

Tool used

Manual Review

Recommendation

To address this issue, the simplest method is to increment the epoch currentEpochsByAsset when RioLRTWithdrawalQueue.queueCurrentEpochSettlement is called. In this case, the epoch will not be settled in a second rebalance call as highlighted in the scenario above.

Duplicate of #4