code-423n4 / 2023-04-eigenlayer-findings

1 stars 1 forks source link

The condition for full withdrawals in the code is different from that in the documentation. #38

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-04-eigenlayer/blob/398cc428541b91948f717482ec973583c9e76232/src/contracts/pods/EigenPod.sol#L354

Vulnerability details

Impact

Detailed description of the impact of this finding. The condition for full withdrawals in the code is different from that in the documentation.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept. The condition in docs for full withdrawal is validator.withdrawableEpoch < executionPayload.slot/SLOTS_PER_EPOCH while in the code its validator.withdrawableEpoch <= executionPayload.slot/SLOTS_PER_EPOCH

    function verifyAndProcessWithdrawal(
        BeaconChainProofs.WithdrawalProofs calldata withdrawalProofs, 
        bytes calldata validatorFieldsProof,
        bytes32[] calldata validatorFields,
        bytes32[] calldata withdrawalFields,
        uint256 beaconChainETHStrategyIndex,
        uint64 oracleBlockNumber
    )
...
        // 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);
        }
    }

src/contracts/pods/EigenPod.sol#L354

Tools Used

Recommended Mitigation Steps

Synchronize them with each other.

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as primary issue

Sidu28 commented 1 year ago

We believe this is an informational-level issue. We failed to update this statement in the higher-level documentation. The code is correct.

c4-sponsor commented 1 year ago

Sidu28 marked the issue as sponsor disputed

c4-sponsor commented 1 year ago

Sidu28 marked the issue as disagree with severity

c4-judge commented 1 year ago

GalloDaSballo changed the severity to QA (Quality Assurance)

GalloDaSballo commented 1 year ago

Great catch, but in lack of an impact am downgrading to QA

Will award extra points

L + 3

c4-judge commented 1 year ago

GalloDaSballo marked the issue as grade-a