fnanni - Execution Layer rewards are lost

[sherlock-admin2]

fnanni

high

Execution Layer rewards are lost

Summary

According to Rio Network Docs: "The Reward Distributor contract (RioLRTRewardDistributor) has the ability to receive ETH via the Ethereum Execution Layer or EigenPod rewards and then distribute those rewards". However, this is only true for EigenPod rewards. Execution Layer rewards are not accounted for and lost.

Vulnerability Detail

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.

The RioLRTRewardDistributor contract relies solely on its receive function to distribute rewards. EL rewards which don't trigger this function are not accounted in the smart contract and there is no way of distributing them.

Impact

Execution Layer rewards are lost.

Code Snippet

Tool used

Manual Review

Recommendation

Add a method to manually distribute EL rewards. For example:

    function distributeRemainingBalance() external {
        uint256 value = address(this).balance;

        uint256 treasuryShare = value * treasuryETHValidatorRewardShareBPS / MAX_BPS;
        uint256 operatorShare = value * operatorETHValidatorRewardShareBPS / MAX_BPS;
        uint256 poolShare = value - treasuryShare - operatorShare;

        if (treasuryShare > 0) treasury.transferETH(treasuryShare);
        if (operatorShare > 0) operatorRewardPool.transferETH(operatorShare);
        if (poolShare > 0) address(depositPool()).transferETH(poolShare);

        emit ETHValidatorRewardsDistributed(treasuryShare, operatorShare, poolShare);
    }

[nevillehuang]

request poc

I need more information/resources for this issue so need to facilitate discussion.

[sherlock-admin3]

PoC requested from @fnanni-0

Requests remaining: 6

[fnanni-0]

@nevillehuang I forgot to link to the Rio docs: https://docs.rio.network/rio-architecture/deposits-and-withdraws/reward-distributor. Can we get @solimander input here? Reading this issue again, there is a chance I misunderstood the docs. Is the Reward Distributor contract expected to be able to receive Execution Layer rewards, i.e. be set as blocks fee_recipient address?

If the answer is yes:

From the Solidity docs: "A contract without a receive Ether function can receive Ether as a recipient of a coinbase transaction". The recipient of a coinbase transaction post-merge is the address defined by the block proposer in the fee_recipient field of the Execution Payload. According to https://eth2book.info/capella/annotated-spec/ :

fee_recipient is the Ethereum account address that will receive the unburnt portion of the transaction fees (the priority fees). This has been called various things at various times: the original Yellow Paper calls it beneficiary; EIP-1559 calls it author. In any case, the proposer of the block sets the fee_recipient to specify where the appropriate transaction fees for the block are to be sent. Under proof of work this was the same address as the COINBASE address that received the block reward. 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.

As an example go to etherscan and select any block recently produced. Check the fee recipient address. Check how its ETH balance is updated ("credited") at every transaction included in the block even though there is no explicit transaction to the fee recipient address (for example, balance update of beaverbuild here).

[solimander]

Our operators will run MEV-Boost, which sets the fee recipient to the builder, who then transfers rewards to the proposer, which triggers the receive function.

However, it seems worth adding a function to manually push rewards just in case. How does that affect severity here?

[fnanni-0]

@solimander I have a few questions:

1. If mev-boost isn't available for a given block (for example there's a timeout), doesn't mev-boost fallback to a validator's local block proposal? See this comment about Teku's client for instance (or Teku's docs). In such case, fee_recipient would be the proposer address, not the builder's address.
2. The flow you described is the current standarized payment method for mev-boost. I wonder if this could change or if there are other builder networks handling this differently. If so, I think it's risky to assume that the proposer address will always receive rewards through direct transfers.
3. Isn't it likely that Ethereum upgrades in the future to better support Proposer-Builder Separation? If this happens, there's a chance the proposer address gets credited, not triggering the receive function.

[solimander]

If mev-boost isn't available for a given block (for example there's a timeout), doesn't mev-boost fallback to a validator's local block proposal? See flashbots/mev-boost#222 (comment) about Teku's client for instance (or Teku's docs). In such case, fee_recipient would be the proposer address, not the builder's address.

I'm unsure, but that'd make sense. I'll be adding a function to manually split and push rewards regardless.

[nevillehuang]

This issue seems out of scope and hinges on external admin integrations. But leaving open for escalation period

[sherlock-admin4]

The protocol team fixed this issue in the following PRs/commits:
https://github.com/rio-org/rio-sherlock-audit/pull/6

[10xhash]

Escalate

The info that the RioLRTRewardDistributor is meant to receive the execution layer rewards is not mentioned anywhere. The mentioned docs state The Reward Distributor contract (RioLRTRewardDistributor) has the ability to receive ETH via the Ethereum Execution Layer or EigenPod rewards and then distribute those rewards, which only indicates that the RioLRTRewardDistributor should be able to receive ETH which it does.

[sherlock-admin2]

Escalate

The info that the RioLRTRewardDistributor is meant to receive the execution layer rewards is not mentioned anywhere. The mentioned docs state The Reward Distributor contract (RioLRTRewardDistributor) has the ability to receive ETH via the Ethereum Execution Layer or EigenPod rewards and then distribute those rewards, which only indicates that the RioLRTRewardDistributor should be able to receive ETH which it does.

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[fnanni-0]

I disagree. The Reward Distributor is expected to receive and distribute rewards. “the ability to receive ETH via the Ethereum Execution Layer or EigenPod rewards”, in this context, seems to mean Execution Layer rewards, which the sponsor confirmed. EL rewards are credited, not transferred, and therefore cannot be currently distributed.

Following up on the prior discussion with the sponsor, regarding operators' obligation to use mev-boost: builders replace the proposer's fee recipient address with their own and directly transfer funds to the former, triggering the receive function. mev-boost is not mentioned in Rio’s docs, but still EL rewards will sometimes be lost:

1. if the builder network becomes unavailable through the configured relays for whatever reason, validator clients fallback to local block production in order to avoid missing a block proposal.
2. mev-boost accepts a minimum bid parameter (see this and this). If set, the process will fallback to local block production every time bids from the builder network don’t surpass this threshold.
3. clients give the option to fallback to local block production when the builder bid represents less than X% of the locally produced block profits. See for example Teku builder-bid-compare-factor, Lighthouse builder-boost-factor or Prysm local-block-value-boost.

For operators aiming for optimal performance, settings 1, 2, and 3 are crucial. RioLRTRewardDistributor assumes the fee recipient role in these instances, causing priority fees and MEV funds to get stuck.

[solimander]

We only mention MEV-Boost in our operator docs.

Valid in that execution layer rewards would be stuck in the fallback case.

[nevillehuang]

I believe @10xhash is correct, this seems to be out of scope and invalid based on the following sherlock rule unless otherwise stated in documentation, which is not present.

14. Loss of airdrops or liquidity fees or any other rewards that are not part of the original protocol design is not considered a valid high/medium. Example

[fnanni-0]

@nevillehuang could you elaborate on why do you consider EL rewards sent to RioLRTRewardDistributor rewards that are not part of the original protocol design?

The documentation (here and here) mentions that EL rewards are expected and RioLRTRewardDistributor must handle them. In addition, it is stated in the operators docs that whitelisted operators must make sure this happens (otherwise I guess the RioDAO would delist operators who disobey?). Third, go to the FAQs section in https://goerli.rio.network/ and you will see:

How often do rewards compound/turnover?

Restaking rewards (less fees) are deposited into the LRT’s deposit pool when the daily epoch ends. These rewards are based on the Ethereum consensus layer -->and the execution layer rewards<--.

Furthermore, rewards expected to be received by RioLRTRewardDistributor (EL rewards included) are distributed to the treasury, the operatorRewardPool and the depositPool. This means that EL rewards become:

1. profit for RioDAO.
2. profit for operators.
3. yield for the Rio protocol and potentially financial source of new validators that will allow even more yield into Rio. This seems like an important feature and incentive for users staking in Rio.

There are many places in which it is explicitly or implicitly pointed at the fact that Execution Layer rewards are part of Rio's protocol rewards and the sponsor confirmed this.

[nevillehuang]

@fnanni-0 You are correct, thanks for pointing me to the right resource. I believe this issue is valid and should remain as it is.

[Czar102]

I believe that Execution Layer rewards are a part of the initial design, but I don't see why would the RioLRTRewardDistributor be thought to be able to receive these rewards directly. I will quote the same source as above:

Restaking rewards (less fees) are deposited into the LRT’s deposit pool when the daily epoch ends. These rewards are based on the Ethereum consensus layer and the execution layer rewards.

Please note that a word "depositing" is used, which implies that the deposit logic is triggered during these operations, and the balance isn't just incremented.

Aside from that, the MEV Bosst is mentioned in the docs:

To obtain the Maximal Extractable Value (MEV) from a block, the block builder can choose to auction block space to block builders in a competitive marketplace with software like MEV boost, further increasing potential rewards.

In order to keep this issue valid, there needs to be an expectation of RioLRTRewardDistributor to receive the Execution Layer rewards directly. @fnanni-0 can you let me know where is it communicated?

[nevillehuang]

@Czar102 Agree with your reasoning, I think it was not explicitly stated that execution layer rewards are expected to be received directly, as mentioned here

[fnanni-0]

@Czar102 I think this is where it's the clearest. The Reward Distributor contract is mentioned and the instruction to set it as the fee recipient means that it is expected to receive execution layer rewards directly.

Post-Approval Requirements

Once approved by the RioDAO, please complete the following steps:

Ethereum Validators

Execution Layer Rewards Configuration

Set the fee recipient for your validators to the Reward Distributor contract. This is NOT the same as the Withdrawal Credentials address.

Note that even with mev-boost as a requirement (which the docs don't seem to say it's a must), setting fee recipient to the Reward Distributor contract means that in fallback cases it will receive EL rewards directly, as explain in my previous comment.

In case it matters, the documentation provided in the contest readme (https://docs.rio.network/) references the operator docs, quoted above, in the section CONTRACTS AND TOOLING --> Tooling --> Rio Operator Guide.

Regarding,

but I don't see why would the RioLRTRewardDistributor be thought to be able to receive these rewards directly

I guess the operator docs are enough, but let me elaborate a bit more. EigenLayer doesn't handle execution layer rewards, so it's evident that they must be handled differently. There are no other contracts in Rio protocol for this purpose except for RioLRTRewardDistributor and nowhere it is explained that there could be a special method for handling EL rewards before sending them to RioLRTRewardDistributor. The most straight forward, intuitive way is to simply send the rewards directly.

Here are all the other chunks of documentation and comments linked in this discussion that seem to me to clearly state that EL rewards are or could be received directly by the RioLRTRewardDistributor:

The Reward Distributor contract (RioLRTRewardDistributor) has the ability to receive ETH via the Ethereum Execution Layer or EigenPod rewards and then distribute those rewards.

ETH staking consensus rewards are claimed from EigenPods [...]. ETH staking execution rewards flow directly through the reward distributor.

Rewards

Participating in restaking generates rewards in a number of forms outlined below. When rewards are received by the RewardDistributor contract in a form other than ETH, [...].

EigenLayer Rewards and EigenLayer Points

[...]

Native Staking Rewards

[...]

Consensus Layer Rewards

[...]

--> Execution Layer Rewards <--

[...]

Restaking rewards [quoted above] (less fees) are deposited into the LRT’s deposit pool when the daily epoch ends. These rewards are based on the Ethereum consensus layer and the execution layer rewards.

Regarding the last quote, you wrote:

Please note that a word "depositing" is used, which implies that the deposit logic is triggered during these operations, and the balance isn't just incremented.

You are right in the sense that the deposit logic is referenced, but I think the link might be incorrect. "deposited when the daily epoch ends" is contradictory, since the deposit flow and the rebalance flow are two separate things. But "depositing" is use in the context of the Deposit Pool contract receiving rewards which is well documented, because the Reward Distributor contract sends ETH to it which can be withdrawn/staked during rebalancing.

[Czar102]

Great points, @fnanni-0.

@nevillehuang I think the message you quoted actually supports @fnanni-0's point, am I understanding correctly?

ETH staking execution rewards flow directly through the reward distributor.

@fnanni-0 does Rio have control over when the Execution Layer Rewards are sent if they are sent directly? I think lack of my knowledge about it didn't allow me to fully understand the meaning of the last quote from the docs about daily deposits of rewards.

[fnanni-0]

@fnanni-0 does Rio have control over when the Execution Layer Rewards are sent if they are sent directly?

@Czar102 I don't think so. They are sent to the Reward Distributor directly when a validator controlled by an operator registered in Rio is selected as block proposer and one of these three scenarios happens (or if mev-boost is not used). Does this answer your question?

about daily deposits of rewards

In case it wasn't clear, the Deposit Pool should receive the rewards from the Reward Distributor at any time, independently of the rebalance execution, like all deposits. It's the rebalancing of these deposits (rewards included) that happens daily. Anyway, this doesn't seem very relevant here.

[Czar102]

I see, thank you for this elaboration. It seems that this is indeed a valid concern.

@solimander @nevillehuang @10xhash @mstpr if you still diagree, please elaborate what are we misunderstanding.

Planning to reject the escalation and leave the issue as is.

[Czar102]

Result:
Medium
Unique

[sherlock-admin3]

Escalations have been resolved successfully!

Escalation status:

• 10xhash: rejected

[10xhash]

The protocol team fixed this issue in PR/commit rio-org/rio-sherlock-audit#6.

Fixed
A function is added to manually distribute the ETH rewards rather than the receive() function itself distributing the rewards

[sherlock-admin4]

The Lead Senior Watson signed off on the fix.