code-423n4 / 2024-04-renzo-findings

11 stars 8 forks source link

activateRestaking() and startDelayedWithdrawUnstakedETH() will always revert #370

Closed howlbot-integration[bot] closed 4 months ago

howlbot-integration[bot] commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/Delegation/OperatorDelegator.sol#L101-L103 https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/Delegation/OperatorDelegator.sol#L459-L463

Vulnerability details

Impact

The OperatorDelegator.activateRestaking() and OperatorDelegator.startDelayedWithdrawUnstakedETH() will always revert when called. This is due to a clause in the eigenPod contract which enables restaking by default during initialization.

The eigenpod manager initializes the pod, which inturn sets eigenpod.hasRestaked to true. Which is checked in both eigenPod.activateRestaking(); and eigenPod.withdrawBeforeRestaking();. Therefore withdrawBeforeRestaking can't be used to sweep eth from the Pod contract.

Proof of Concept

The eigenpod contract can be found in: https://github.com/Layr-Labs/eigenlayer-contracts?tab=readme-ov-file#eigenpods-1 The modifier below causes the restrictions

    modifier hasNeverRestaked() {
        require(!hasRestaked, "EigenPod.hasNeverRestaked: restaking is enabled");
        _;
    }

Tools Used

Manual Review, Josephdara

Recommended Mitigation Steps

Remove both functions from the contract completely as withdrawals cannot be made through them

Assessed type

Error

C4-Staff commented 5 months ago

CloudEllie marked the issue as duplicate of #571

c4-judge commented 5 months ago

alcueca marked the issue as not a duplicate

jatinj615 commented 4 months ago

The withdrawBeforeRestaking function was getting used in M1 version of EigenLayer to withdraw funds, which then got migrated to M2 when restaking got enabled and according to EigenLayer migration instructions PodOwner(OperatorDelegator contract in our case) has to call activateRestaking only once to migrate from M1->M2. After M2 upgrade withdrawals are verified by verifyAndProcessWithdrawals. The statement is correct that the functions will revert. But the functions were kept to provide seamless migration from M1 -> M2 upgrade for EigenLayer pods.

alcueca commented 4 months ago

A bit of natspec would have helped, or the warden could have asked what was the purpose of the functions.

c4-judge commented 4 months ago

alcueca marked the issue as unsatisfactory: Invalid