Wrong accounting of ethBalanceInUnverifiedValidators when validating withdraw credentials

Summary

After calling verifyWithdrawalCredentials the value of ethBalanceInUnverifiedValidators is decreased exactly 32 ether for each validator verified. But this amount does not necesarily have to be equal to the amount recorded in the EigenPodManager of Eigenlayer, potentially leading to an accounting mismatch, because ethBalanceInUnverifiedValidators is used in the computation of the LRT TVL.

Vulnerability Detail

When verifyWithdrawalCredentials is invoked, there is a call to decreaseUnverifiedValidatorETHBalance passing in the number of validators times ETH_DEPOSIT_SIZE which is hardcoded to 32 ether:

function verifyWithdrawalCredentials(
    uint8 operatorId,
    uint64 oracleTimestamp,
    IBeaconChainProofs.StateRootProof calldata stateRootProof,
    uint40[] calldata validatorIndices,
    bytes[] calldata validatorFieldsProofs,
    bytes32[][] calldata validatorFields
) external onlyOperatorManagerOrProofUploader(operatorId) {
    OperatorDetails storage operator = s.operatorDetails[operatorId];
    IRioLRTOperatorDelegator(operator.delegator).verifyWithdrawalCredentials(
        oracleTimestamp, stateRootProof, validatorIndices, validatorFieldsProofs, validatorFields
    );

    // Once verified, shares are tracked as EigenPod shares.
    assetRegistry().decreaseUnverifiedValidatorETHBalance(validatorIndices.length * ETH_DEPOSIT_SIZE);

    emit OperatorWithdrawalCredentialsVerified(operatorId, oracleTimestamp, validatorIndices);
}

verifyWithdrawalCredentials in RioLRTOperatorDelegator eventually calls verifyWithdrawalCredentials in the Eigen Pod, which, as can be seen below, records in the Eigen Pod manager the total amount to be restaked.

https://github.com/Layr-Labs/eigenlayer-contracts/blob/6de01c6c16d6df44af15f0b06809dc160eac0ebf/src/contracts/pods/EigenPod.sol#L344

And this amount does not have to be necesarily equal to a multiple of 32 ether (validators can be slashed, for instance).

So the value of the TVL of the LRT can be affected and not reflect it's real total value locked. The flow to calculate the TVL is:

RioLRTAssetRegistry:getTVL |-> RioLRTAssetRegistry:getTVLForAsset |-> RioLRTAssetRegistry:getTotalBalanceForAsset |-> RioLRTAssetRegistry:getETHBalanceInEigenLayer

In this last call we can see ethBalanceInUnverifiedValidators is used in the calculation, so if it was wrong it would certainly affect the TVL amount.

function getETHBalanceInEigenLayer() public view returns (uint256 balance) {
    balance = ethBalanceInUnverifiedValidators;

    IRioLRTOperatorRegistry operatorRegistry_ = operatorRegistry();
    uint8 endAtID = operatorRegistry_.operatorCount() + 1; // Operator IDs start at 1.
    for (uint8 id = 1; id < endAtID; ++id) {
        balance += operatorDelegator(operatorRegistry_, id).getETHUnderManagement();
    }
}

As getTVL is used in the calls to convertFromUnitOfAccountToRestakingTokens and convertToUnitOfAccountFromRestakingTokens, a wrong value would propagate and produce, for instance, that a call to depositETH would mint an incorrect amount of the LRT to the depositor.

Impact

High, accounting mismatch can lead to loss of funds to stakers.

Code Snippet

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

Tool used

Manual review.

Recommendation

The call to decreaseUnverifiedValidatorETHBalance in verifyWithdrawalCredentials should be done passing the actual amount recorded in Eigenlayer.

Duplicate of #235

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

neumo - Wrong accounting of ethBalanceInUnverifiedValidators when validating withdraw credentials #357