Skip to content
This repository has been archived by the owner on Sep 8, 2024. It is now read-only.

zzykxx - The protocol can't receive rewards because of low gas limits on ETH transfers #185

Open
sherlock-admin opened this issue Mar 7, 2024 · 3 comments
Labels
Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed

Comments

@sherlock-admin
Copy link
Contributor

sherlock-admin commented Mar 7, 2024

zzykxx

high

The protocol can't receive rewards because of low gas limits on ETH transfers

Summary

The hardcoded gas limit of the Asset::transferETH() function, used to transfer ETH in the protocol, is too low and will result unwanted reverts.

Vulnerability Detail

ETH transfers in the protocol are always done via Asset::transferETH(), which performs a low-level call with an hardcoded gas limit of 10_000:

(bool success,) = recipient.call{value: amount, gas: 10_000}('');
if (!success) {revert ETH_TRANSFER_FAILED();}

The hardcoded 10_000 gas limit is not high enough for the protocol to be able receive and distribute rewards. Rewards are currently only available for native ETH, an are received by Rio via:

  • Partial withdrawals
  • ETH in excess of 32ETH on full withdrawals

The flow to receive rewards requires two steps:

  1. An initial call to EigenPod::verifyAndProcessWithdrawals(), which queues a withdrawal to the Eigenpod owner: an RioLRTOperatorDelegator instance
  2. A call to DelayedWithdrawalRouter::claimDelayedWithdrawals().

The call to DelayedWithdrawalRouter::claimDelayedWithdrawals() triggers the following flow:

  1. ETH are transferred to the RioLRTOperatorDelegator instance, where the receive() function is triggered.
  2. The receive() function of RioLRTOperatorDelegator transfers ETH via Asset::transferETH() to the RioLRTRewardDistributor, where another receive() function is triggered.
  3. The receive() function of RioLRTRewardDistributor transfers ETH via Asset::transferETH() to the treasury, the operatorRewardPool and the RioLRTDepositPool.

The gas is limited at 10_000 in step 2 and is not enough to perform step 3, making it impossible for the protocol to receive rewards and leaving funds stuck.

POC

Add the following imports to RioLRTOperatorDelegator.t.sol:

import {IRioLRTOperatorRegistry} from 'contracts/interfaces/IRioLRTOperatorRegistry.sol';
import {RioLRTOperatorDelegator} from 'contracts/restaking/RioLRTOperatorDelegator.sol';
import {CredentialsProofs, BeaconWithdrawal} from 'test/utils/beacon-chain/MockBeaconChain.sol';

then copy-paste:

function test_outOfGasOnRewards() public {
    address alice = makeAddr("alice");
    uint256 initialBalance = 40e18;
    deal(alice, initialBalance);
    vm.prank(alice);
    reETH.token.approve(address(reETH.coordinator), type(uint256).max);

    //->Operator delegator and validators are added to the protocol
    uint8 operatorId = addOperatorDelegator(reETH.operatorRegistry, address(reETH.rewardDistributor));
    RioLRTOperatorDelegator operatorDelegator =
        RioLRTOperatorDelegator(payable(reETH.operatorRegistry.getOperatorDetails(operatorId).delegator));

    //-> Alice deposits ETH in the protocol
    vm.prank(alice);
    reETH.coordinator.depositETH{value: initialBalance}();
    
    //-> Rebalance is called and the ETH deposited in a validator
    vm.prank(EOA, EOA);
    reETH.coordinator.rebalance(ETH_ADDRESS);

    //-> Create a new validator with a 40ETH balance and verify his credentials.
    //-> This is to "simulate" rewards accumulation
    uint40[] memory validatorIndices = new uint40[](1);
    IRioLRTOperatorRegistry.OperatorPublicDetails memory details = reETH.operatorRegistry.getOperatorDetails(operatorId);
    bytes32 withdrawalCredentials = operatorDelegator.withdrawalCredentials();
    beaconChain.setNextTimestamp(block.timestamp);
    CredentialsProofs memory proofs;
    (validatorIndices[0], proofs) = beaconChain.newValidator({
        balanceWei: 40 ether,
        withdrawalCreds: abi.encodePacked(withdrawalCredentials)
    });
    
    //-> Verify withdrawal crendetials
    vm.prank(details.manager);
    reETH.operatorRegistry.verifyWithdrawalCredentials(
        operatorId,
        proofs.oracleTimestamp,
        proofs.stateRootProof,
        proofs.validatorIndices,
        proofs.validatorFieldsProofs,
        proofs.validatorFields
    );

    //-> Process a full withdrawal, 8ETH (40ETH - 32ETH) will be queued withdrawal as "rewards"
    verifyAndProcessWithdrawalsForValidatorIndexes(address(operatorDelegator), validatorIndices);

    //-> Call `claimDelayedWithdrawals` to claim the withdrawal
    delayedWithdrawalRouter.claimDelayedWithdrawals(address(operatorDelegator), 1); //❌ Reverts for out-of-gas
}

Impact

The protocol is unable to receive rewards and the funds will be stucked.

Code Snippet

Tool used

Manual Review

Recommendation

Remove the hardcoded 10_000 gas limit in Asset::transferETH(), at least on ETH transfers where the destination is a protocol controlled contract.

@sherlock-admin4
Copy link

sherlock-admin4 commented Mar 13, 2024

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

@sherlock-admin2 sherlock-admin2 added Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed labels Mar 13, 2024
@github-actions github-actions bot added Medium A valid Medium severity issue Has Duplicates A valid issue with 1+ other issues describing the same vulnerability labels Mar 16, 2024
@sherlock-admin2 sherlock-admin2 changed the title Rural Tweed Lemur - The protocol can't receive rewards because of low gas limits on ETH transfers zzykxx - The protocol can't receive rewards because of low gas limits on ETH transfers Mar 26, 2024
@sherlock-admin2 sherlock-admin2 added the Reward A payout will be made for this issue label Mar 26, 2024
@10xhash
Copy link

10xhash commented Apr 15, 2024

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

Fixed
Removed the 10k gas limit

@sherlock-admin4
Copy link

The Lead Senior Watson signed off on the fix.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed
Projects
None yet
Development

No branches or pull requests

4 participants