sherlock-audit / 2024-02-rio-network-core-protocol-judging

4 stars 4 forks source link

g - Attacker can block deposits to Eigenlayer and prevent yields #95

Closed sherlock-admin3 closed 7 months ago

sherlock-admin3 commented 7 months ago

g

medium

Attacker can block deposits to Eigenlayer and prevent yields

Summary

Anyone can fully withdraw ETH from an operator in Eigenlayer. This exits validators and once validators are exited, they can no longer be allocated ETH to. This is what enables anyone to prevent the protocol from gaining yields.

Vulnerability Detail

Anyone can fully withdraw ETH from an Eigepod via scraping in the Operator Delegator.

ref: https://github.com/sherlock-audit/2024-02-rio-network-core-protocol/blob/main/rio-sherlock-audit/contracts/restaking/RioLRTOperatorDelegator.sol#L160-L167

function scrapeExcessFullWithdrawalETHFromEigenPod() external {
    uint256 ethWithdrawable = eigenPod.withdrawableRestakedExecutionLayerGwei().toWei();
    uint256 ethQueuedForWithdrawal = getETHQueuedForWithdrawal();
    if (ethWithdrawable <= ethQueuedForWithdrawal + MIN_EXCESS_FULL_WITHDRAWAL_ETH_FOR_SCRAPE) {
        revert INSUFFICIENT_EXCESS_FULL_WITHDRAWAL_ETH();
    }
    _queueWithdrawalForOperatorExitOrScrape(BEACON_CHAIN_STRATEGY, ethWithdrawable - ethQueuedForWithdrawal);
}

Once a validator has been deposited into, it can not be deposited into again even after it has exited. An attacker can then execute the following steps to block the LRT from restaking and gaining any yields from Eigenlayer:

ref: https://github.com/sherlock-audit/2024-02-rio-network-core-protocol/blob/main/rio-sherlock-audit/contracts/restaking/RioLRTOperatorRegistry.sol#L431-L449

uint256 unallocatedConfirmedKeys = validators.confirmed - validators.deposited;
if (unallocatedConfirmedKeys == 0) {
    skippedOperators[skippedOperatorCount++] = heap.extractMin();
    continue;
}

// Each allocation is a 32 ETH deposit. We can only allocate up to the number of unallocated confirmed keys.
uint256 updatedAllocation;
{
    uint256 newDepositAllocation = FixedPointMathLib.min(
        FixedPointMathLib.min(validators.cap - activeDeposits, unallocatedConfirmedKeys), remainingDeposits
    );

    // ... snip ...
    operator.validatorDetails.deposited += uint40(newDepositAllocation);

The code above is in OperatorRegistry::allocateETHDeposits() and it shows how the protocol can no longer deposit ETH once there are no more unallocated validators (unallocatedConfirmedKeys is 0.)

Impact

In this scenario, the LRT will not gain any staking yields since the attacker has effectively blocked the LRT from depositing into Eigenlayer. The LRT is no longer able to restake until more validator keys are added. However, the attacker can just repeat the attack.

Code Snippet

Tool used

Manual Review

Recommendation

Consider restricting access to scrapeExcessFullWithdrawalETHFromEigenPod() in the Operator Delegator. Allow only the Operator Manager and/or the Owner to access it.

Duplicate of #144

solimander commented 7 months ago

Invalid - Misunderstanding around how scrapeExcessFullWithdrawalETHFromEigenPod works. This function can only scrape ETH that has already exited from validators and is not being used in existing withdrawals. It can't force operators to exit validators.