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

Risk of wasting ETH balance in DepositQueue #505

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

Risk of wasting ETH balance in DepositQueue #505

howlbot-integration bot opened this issue May 10, 2024 · 5 comments
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue grade-a Q-30 QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax 🤖_37_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#L283-L289

Vulnerability details

Impact

The administrator has the ability to drain the ETH balance of the DepositQueue contract by calling certain functions with a high gas price. This can lead to:

  • TVL Waste: Draining the contract's balance wastes the Total Value Locked (TVL) in the protocol.
  • Lower Token Value: This imbalance can lower the value of the ezETH token (even less than 1 ETH).
  • Deficit in Operator Delegator Balance: Insufficient balance can lead to a deficit in supplying operator delegators, affecting protocol operations.

Proof of Concept

  1. Gas Refund Mechanism: When the administrator, with the role NATIVE_ETH_RESTAKE_ADMIN, calls the stakeEthFromQueue or stakeEthFromQueueMulti functions, any consumed gas is refunded to the msg.sender.
    function stakeEthFromQueue(
        IOperatorDelegator operatorDelegator,
        bytes calldata pubkey,
        bytes calldata signature,
        bytes32 depositDataRoot
    ) external onlyNativeEthRestakeAdmin {
        uint256 gasBefore = gasleft();
        // Send the ETH and the params through to the restake manager
        restakeManager.stakeEthInOperatorDelegator{ value: 32 ether }(
            operatorDelegator,
            pubkey,
            signature,
            depositDataRoot
        );

        emit ETHStakedFromQueue(operatorDelegator, pubkey, 32 ether, address(this).balance);

        // Refund the gas to the Admin address if enough ETH available
        _refundGas(gasBefore);
    }

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

    function stakeEthFromQueueMulti(
        IOperatorDelegator[] calldata operatorDelegators,
        bytes[] calldata pubkeys,
        bytes[] calldata signatures,
        bytes32[] calldata depositDataRoots
    ) external onlyNativeEthRestakeAdmin nonReentrant {
        uint256 gasBefore = gasleft();
        // Verify all arrays are the same length
        if (
            operatorDelegators.length != pubkeys.length ||
            operatorDelegators.length != signatures.length ||
            operatorDelegators.length != depositDataRoots.length
        ) revert MismatchedArrayLengths();

        // Iterate through the arrays and stake each one
        uint256 arrayLength = operatorDelegators.length;
        for (uint256 i = 0; i < arrayLength; ) {
            // Send the ETH and the params through to the restake manager
            restakeManager.stakeEthInOperatorDelegator{ value: 32 ether }(
                operatorDelegators[i],
                pubkeys[i],
                signatures[i],
                depositDataRoots[i]
            );

            emit ETHStakedFromQueue(
                operatorDelegators[i],
                pubkeys[i],
                32 ether,
                address(this).balance
            );

            unchecked {
                ++i;
            }
        }

        // Refund the gas to the Admin address if enough ETH available
        _refundGas(gasBefore);
    }

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

  1. Gas Refund Source: The refunded gas is paid from the balance of the DepositQueue contract and is calculated based on (initialGas - gasleft()) * tx.gasprice.
    function _refundGas(uint256 initialGas) internal {
        uint256 gasUsed = (initialGas - gasleft()) * tx.gasprice;
        uint256 gasRefund = address(this).balance >= gasUsed ? gasUsed : address(this).balance;
        (bool success, ) = payable(msg.sender).call{ value: gasRefund }("");
        if (!success) revert TransferFailed();
        emit GasRefunded(msg.sender, gasRefund);
    }

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

  1. Risk Scenario: If the admin sets a very high tx.gasprice, it can lead to a substantial drain on the contract's balance. For example the transaction https://etherscan.io/tx/0xc5ae7b766c0186590d4baf648a17b1e59f96c01a540e3b5aee675e87a882555f shows that calling the function stakeEthFromQueueMulti for two operator delegators consumes almost 286745 gas. If the admin sets the gas price in the transation to 1000 Gwei, almost 0.287 ETH will be wasted from the contract. The admin does not gain financially, but it can adversly impacts on the protocol.

Tools Used

Recommended Mitigation Steps

Modify the _refundGas function to allocate only a small percentage of the contract's balance for gas refunds:

function _refundGas(uint256 initialGas) internal {
        uint256 gasUsed = (initialGas - gasleft()) * tx.gasprice;

        uint256 refundPool = address(this).balance / 1000; // %0.1 of the balance is dedicated to the gas refund
        uint256 gasRefund = refundPool >= gasUsed ? gasUsed : refundPool;

        (bool success, ) = payable(msg.sender).call{ value: gasRefund }("");
        if (!success) revert TransferFailed();
        emit GasRefunded(msg.sender, gasRefund);
    }

Assessed type

Context

@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 🤖_37_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-Staff
Copy link

CloudEllie marked the issue as duplicate of #504

@c4-judge
Copy link
Contributor

alcueca marked the issue as not a duplicate

@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 20, 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-judge c4-judge added grade-a and removed grade-a labels May 24, 2024
@c4-judge
Copy link
Contributor

alcueca marked the issue as grade-b

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 grade-a Q-30 QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax 🤖_37_group AI based duplicate group recommendation sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

3 participants