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

12 stars 8 forks source link

Execution Layer rewards are lost #499

Open howlbot-integration[bot] opened 6 months ago

howlbot-integration[bot] commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Deposits/DepositQueue.sol#L158-L184 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Delegation/OperatorDelegator.sol#L494-L525

Vulnerability details

Impact

According to comment in DepositQueue: "The Deposit Queue contract has the ability to receive ETH via the Ethereum Execution Layer.However, this is only true for EigenPod rewards. Execution Layer rewards are not accounted for and lost.

This should receive ETH from scenarios like Execution Layer Rewards and MEV from native staking

Proof of Concept

Execution Layer rewards are not distributed through plain ETH transfers. Instead the balance of the block proposer fee recipient's address is directly updated. If the fee recipient getting the EL rewards is a smart contract, this means that the fallback/receive function is not called. Actually, a smart contract could receive EL rewards even if these functions are not defined.


    /// @dev Handle ETH sent to this contract from outside the protocol - e.g. rewards
    /// ETH will be stored here until used for a validator deposit
@-> /// This should receive ETH from scenarios like Execution Layer Rewards and MEV from native staking
    /// Users should NOT send ETH directly to this contract unless they want to donate to existing ezETH holders
    /// Checks the ETH withdraw Queue and fills up if required
    receive() external payable nonReentrant {
        uint256 feeAmount = 0;
        // Take protocol cut of rewards if enabled
        if (feeAddress != address(0x0) && feeBasisPoints > 0) {
            feeAmount = (msg.value * feeBasisPoints) / 10000;
            (bool success, ) = feeAddress.call{ value: feeAmount }("");
            if (!success) revert TransferFailed();

            emit ProtocolFeesPaid(IERC20(address(0x0)), feeAmount, feeAddress);
        }
        // update remaining rewards
        uint256 remainingRewards = msg.value - feeAmount;
        // Check and fill ETH withdraw buffer if required
        _checkAndFillETHWithdrawBuffer(remainingRewards);

        // Add to the total earned
        totalEarned[address(0x0)] = totalEarned[address(0x0)] + remainingRewards;

        // Emit the rewards event
        emit RewardsDeposited(IERC20(address(0x0)), remainingRewards);
    }

This is only true for EigenPod rewards and there is a receive function OperatorDelegator contract to handle eigenPod rewards. Execution Layer rewards are not accounted for and lost.

    /**
     * @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
        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

Recommended Mitigation Steps

Modify the receive() function in the DepositQueue contract to handle EL rewards. This can be achieved by checking the transaction context and identifying EL reward deposits.

Assessed type

Other

c4-judge commented 6 months ago

alcueca changed the severity to QA (Quality Assurance)

c4-judge commented 6 months ago

alcueca marked the issue as grade-a