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

4 stars 4 forks source link

g - Attacker can maliciously increase exited validators and block withdrawals #94

Closed sherlock-admin closed 7 months ago

sherlock-admin commented 7 months ago

g

high

Attacker can maliciously increase exited validators and block withdrawals

Summary

Attacker can maliciously increase exited validators and block ETH withdrawals from Eigenlayer.

Vulnerability Detail

When ETH is withdrawn from Eigenlayer, the Operator chooses any validators to fully withdraw from. The LRT is unable to specify which validators to exit.

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

Each deallocation will trigger the withdrawal of a 32 ETH deposit. The specific validators to withdraw from are chosen by the software run by the operator.

Because of this, any validator in later indices in the utilization heap can be exited by the operator and an attacker could then abuse reportOutOfOrderValidatorExits() to increase the exited validators count.

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

function reportOutOfOrderValidatorExits(uint8 operatorId, uint256 fromIndex, uint256 validatorCount) external {
    // ... snip ...
   for (uint256 i = 0; i < validatorCount; ++i) {
        Memory.copyBytes(exitedPubKeyBatch, publicKey, i * BLS_PUBLIC_KEY_LENGTH, 0, BLS_PUBLIC_KEY_LENGTH);
        if (pod.validatorStatus(_hashValidatorBLSPubKey(publicKey)) != IEigenPod.VALIDATOR_STATUS.WITHDRAWN) {
            revert VALIDATOR_NOT_EXITED();
        }
    }

    VALIDATOR_DETAILS_POSITION.swapValidatorDetails(operatorId, fromIndex, validators.exited, validatorCount);
    operator.validatorDetails.exited += uint40(validatorCount);

    emit OperatorOutOfOrderValidatorExitsReported(operatorId, validatorCount);
}

The LRT can be put in a bad state by increasing the number of exited validators even when no more validators are exited. Below are the steps that lead to it:

Impact

Some of the ETH depositors will be unable to withdraw their ETH principal from the LRT and the LRT will be insolvent.

This is because increasing the exited validator count by 1 will mean that there is 1 less validator that the protocol can withdraw 32 ETH from. The attack can be repeated every time there are out-of-order validator exits during ETH withdrawals from Eigenlayer which will take out even more ETH from being available for withdrawal.

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

function deallocateETHDeposits(uint256 depositsToDeallocate) external onlyCoordinator returns (uint256 depositsDeallocated, OperatorETHDeallocation[] memory deallocations) {
    // ... snip ...

    bytes memory pubKeyBatch;
    while (remainingDeposits > 0) {
        uint8 operatorId = heap.getMax().id;

        OperatorDetails storage operator = s.operatorDetails[operatorId];
        OperatorValidatorDetails memory validators = operator.validatorDetails;
        uint256 activeDeposits = validators.deposited - validators.exited;

        // Exit early if the operator with the highest utilization rate has no active deposits,
        // as no further deallocations can be made.
        if (activeDeposits == 0) break;
    // ... snip ...
}

Code Snippet

Tool used

Manual Review

Recommendation

Consider restricting access to reportOutOfOrderValidatorExits() to the Operator Manager and/or Owner roles. Another function can also be created with similar functionality but only for handling the case of switching places of exited validators with non-exited validators in the utilization heap. This new function will not increase the exited validators count.

Duplicate of #131

solimander commented 7 months ago

Invalid - While the protocol cannot enforce which operators are exited, it does instruct the operators to exit (earliest deposited validators first). They do not exit at random.