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

12 stars 8 forks source link

Execution Layer rewards are lost #496

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/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/Deposits/DepositQueue.sol#L163

Vulnerability details

Summary

As per the Sponsor:

- Any rewards earned by the protocol should be redeposited (minus a protocol fee).

The Execution Layer rewards are not correctly handled, resulting in breaking this invariant.

Description

In DepositQueue.sol, the receive() function is meant to handle ETH sent from outside this protocol, like the Execution Layer rewards:

    /// @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);
    }

The problem is that Execution Layer rewards are not sent in the usual fashion, like a transfer. Instead, the balance of the proposer of the block is updated directly.

This means that the receive() function will not be triggered because there will not be a direct ETH transfer. This results in the Execution Layer rewards not being able to be distributed and therefor being lost.

Tools used

Manual Review

Recommended Mitigation Steps

Add a function just like DepositQueue.sweepERC20(), that can distribute the ETH tokens in the DepositQueue.sol contract.

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