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

11 stars 8 forks source link

Cut is paid on eigenpod ETH withdrawals through ``verifyAndProcessWithdrawals`` #527

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#L405-L425

Vulnerability details

Impact

The receive() function has the following specs:

//     * @dev If msg.sender is eigenPod then forward ETH to deposit queue without taking cut //

This means that the eth transfers coming from the EigenPod should be sent directly to the deposit queue, however this is not the case, as Withdrawals coming from OperatorDelegator.verifyAndProcessWithdrawals() which calls eigenPod.verifyAndProcessWithdrawals. But this does not send available withdrawals directly from the Eigenpod contract. Instead it sends the ETH through the delayedWithdrawalRouter. Since the ETH transfer is finalized through delayedWithdrawalRouter, it does not skip the necessary step in the receive function.

Proof of Concept

    /**
     * @notice Users should NOT send ETH directly to this contract unless they want to donate to existing ezETH holders.
     *        This is an internal protocol function.
     * @dev Handle ETH sent to this contract - will get forwarded to the deposit queue for restaking as a protocol reward
     * @dev If msg.sender is eigenPod then forward ETH to deposit queue without taking cut (i.e. full withdrawal from beacon chain)
     */
    receive() external payable nonReentrant {
        // check if sender contract is EigenPod. forward full withdrawal eth received

        //@audit it does not skip this step
        if (msg.sender == address(eigenPod)) {
            restakeManager.depositQueue().forwardFullWithdrawalETH{ value: msg.value }();
        } else {
            // considered as protocol reward
            uint256 gasRefunded = 0;
            uint256 remainingAmount = msg.value;
            if (adminGasSpentInWei[tx.origin] > 0) {
                gasRefunded = _refundGas();
                // update the remaining amount
                remainingAmount -= gasRefunded;
                // If no funds left, return
                if (remainingAmount == 0) {
                    return;
                }
            }
            // Forward remaining balance to the deposit queue
            address destination = address(restakeManager.depositQueue());
            (bool success, ) = destination.call{ value: remainingAmount }("");
            if (!success) revert TransferFailed();

            emit RewardsForwarded(destination, remainingAmount);
        }
    }

Tools Used

Manual Review, Josephdara

Recommended Mitigation Steps

Ensure that if msg.sender is the delayedWithdrawalRouter, no fee is paid.

Assessed type

ETH-Transfer

jatinj615 commented 5 months ago

The funds coming from DelayedWithdrawalRouter are meant to be beacon chain partial withdrawals (protocol rewards) from which fee should be deducted. In case of Full withdrawal the funds come from EigenPod itself from which no fee should be deducted as this is the user collateral. Reference - https://github.com/Layr-Labs/eigenlayer-contracts/tree/v0.2.2-holesky-init-deployment/docs#completing-a-withdrawal-as-tokens

c4-judge commented 4 months ago

alcueca marked the issue as unsatisfactory: Invalid