Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Execution Layer rewards are lost #496

Open
howlbot-integration bot opened this issue May 10, 2024 · 2 comments
Open

Execution Layer rewards are lost #496

howlbot-integration bot opened this issue May 10, 2024 · 2 comments
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-498 grade-a QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax 🤖_230_group AI based duplicate group recommendation sufficient quality report This report is of sufficient quality

Comments

@howlbot-integration
Copy link

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

@howlbot-integration howlbot-integration bot added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value 🤖_230_group AI based duplicate group recommendation bug Something isn't working sufficient quality report This report is of sufficient quality labels May 10, 2024
howlbot-integration bot added a commit that referenced this issue May 10, 2024
@c4-judge c4-judge added downgraded by judge Judge downgraded the risk level of this issue QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax and removed 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value labels May 17, 2024
@c4-judge
Copy link
Contributor

alcueca changed the severity to QA (Quality Assurance)

@c4-judge
Copy link
Contributor

alcueca marked the issue as grade-a

@C4-Staff C4-Staff reopened this Jun 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-498 grade-a QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax 🤖_230_group AI based duplicate group recommendation sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

4 participants