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

Renzo protocol Execution Layer Rewards are lost #500

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

Renzo protocol Execution Layer Rewards are lost #500

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 Q-32 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/main/contracts/Deposits/DepositQueue.sol#L163

Vulnerability details

Impact

In the contest details, the protocol mentions the following regarding Protocol Fees:

"Native ETH earned from outside EigenLayer (such as Execution Layer Rewards or MEV) will be sent to the DepositQueue receive() function. The protocol will then forward a configured fee percentage to an external address."

However, Execution Layer Rewards are not transferred, but rather credited. This means that the DepositQueue::receive() function will not trigger when these rewards are earned, and rewards will be lost.

Proof of Concept

DepositQueue::receive
https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Deposits/DepositQueue.sol#L163

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

As we can observe from the function and dev comments, it is expected that this function will trigger when Execution Layer rewards are received.

However, that is not the case. Execution Layer Rewards are credited rather than transferred:

"Under proof of stake, the block reward is credited to the validator's beacon chain balance, and the transaction fees are credited to the fee_recipient Ethereum address".

Source: https://eth2book.info/capella/annotated-spec/#:~:text=fee_recipient%20is%20the,fee_recipient%20Ethereum%20address

The receive() function will not trigger and rewards will be lost.

Tools Used

Manual Review.

Recommended Mitigation Steps

Add a function to the DepositQueue contract to distribute Execution Layer rewards from the contract.

Assessed type

ETH-Transfer

@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
@C4-Staff C4-Staff added the Q-32 label 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 Q-32 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