Renzo - gjaldon's results

Lines of code

https://github.com/Layr-Labs/eigenlayer-contracts/blob/15b679b08e7a3589ff83a5e84076ca4e0a00ec0b/src/contracts/pods/DelayedWithdrawalRouter.sol#L100-L105 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Oracle/RenzoOracle.sol#L123-L164 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/RestakeManager.sol#L274-L358 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Delegation/OperatorDelegator.sol#L405-L424 https://github.com/Layr-Labs/eigenlayer-contracts/blob/15b679b08e7a3589ff83a5e84076ca4e0a00ec0b/src/contracts/pods/DelayedWithdrawalRouter.sol#L127-L137 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Delegation/OperatorDelegator.sol#L432-L437 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Delegation/OperatorDelegator.sol#L459-L463

Vulnerability details

There are Native ETH Restake Admin operations in the Operator Delegator that send funds to the Delayed Withdrawal Router.

1. withdrawNonBeaconChainETHBalanceWei()
2. startDelayedWithdrawUnstakedETH()
3. verifyAndProcessWithdrawals()
4. startDelayedWithdrawUnstakedETH()
5. activateRestaking()

Once there are funds in the Delayed Withdrawal Router, anyone can claim these withdrawals after the set delay. When the protocol computes for its TVL, it does not include any of its Operator Delegators' ETH that is in the Delayed Withdrawal Router. This presents an MEV opportunity.

Impact

Users can extract value at the expense of other depositors by sandwiching the claimDelayedWithdrawals() call whenever funds are in the Withdrawal Router.

Proof of Concept

Consider the following scenario:

1. Funds are sent to the Delayed Withdrawal Router by the Native ETH Restake Admin.
2. Once the delayed withdrawal can be claimed, a user can do the following in:

• Deposit tokens to mint as much ezETH as possible.
• Claim the delayed withdrawal in the Delayed Withdrawal Router.
• Withdraw and eventually claim using as much ezETH as possible.

Tools Used

Manual Review, Foundry

Recommended Mitigation Steps

Consider including the total amount in delayed withdrawals and the nonBeaconChainETHBalanceWei for each pod as part of the TVL calculation. Delayed Withdrawal Router's getUserDelayedWithdrawals() can be used for getting the delayed withdrawals.

Assessed type

MEV

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/RestakeManager.sol#L274-L358 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Oracle/Mantle/METHShim.sol#L80 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Oracle/RenzoOracle.sol#L75

Vulnerability details

There are validator penalties and slashings that can decrease the value of staked ETH. This can affect all of Renzo's supported collateral tokens, wbETH, mETH, stETH and ETH.

There is typically a delay between the update of an Oracle and the penalty. In mETH, for example:

If a major slashing event occurs for the protocol's validators, which causes a large loss of ETH controlled by the protocol, the exchange rate will not reflect this slashing until the next Oracle update. As the Oracle is configured to run every 8 hours, it is possible for another party to detect the slashing event, and request an unstake of all of their ETH at an elevated exchange rate before the oracle data "catches up" and the correct rate is quoted.

The described issue above has been mitigated in mETH. However, the issue still applies to Renzo.

The Oracle update in mETH is done every 8 hours. During a major slashing event and before the next Oracle update, depositors can withdraw their ETH from Renzo. The withdrawal is done while TVL has not yet decreased due to the slashing. Users unable to withdraw before the huge decrease in TVL will shoulder the losses of the slashing event.

Impact

This leads to a loss of ETH for all remaining depositors and would cause a rush to exit the protocol. The damage is worse when the assets withdrawn are unaffected by the slashing.

Proof of Concept

The TVL of the protocol is used for computing the minting and withdrawal amounts and is calculated with:

totalTVL = sum(operatorTVL) + DepositQueue ETH Balance + WithdrawQueue TVL

A decrease in the price of any of its collateral tokens will decrease Renzo's TVL.

In the case of mETH, its price is fetched from an Oracle which calls methStaking.mETHToETH(1 ether) to get its price.

ref: mETH Staking contract on mainnet

    function mETHToETH(uint256 mETHAmount) public view returns (uint256) {
        if (mETH.totalSupply() == 0) {
            return mETHAmount;
        }
        return Math.mulDiv(mETHAmount, totalControlled(), mETH.totalSupply());
    }

    /// @notice The total amount of ETH controlled by the protocol.
    /// @dev Sums over the balances of various contracts and the beacon chain information from the oracle.
    function totalControlled() public view returns (uint256) {
        OracleRecord memory record = oracle.latestRecord();
        uint256 total = 0;
        total += unallocatedETH;
        total += allocatedETHForDeposits;
        total += totalDepositedInValidators - record.cumulativeProcessedDepositAmount;
        total += record.currentTotalValidatorBalance;
        total += unstakeRequestsManager.balance();
        return total;
    }

Consider the following scenario:

1. A major slashing event affects mETH's validators.
2. Depositors aware that Renzo holds mETH as one of its assets withdraws ETH for their ezETH before mETH's oracle update happens.
3. Depositors that withdrew before the Oracle update sidestep the validator penalties.
4. After the Oracle update, the remaining depositors shoulder the full loss caused by the slashing.

Tools Used

Manual Review, Foundry

Recommended Mitigation Steps

Consider adding a withdrawal delay when a major slashing event greatly decreases the protocol TVL. Another option is to add pause functionality to withdrawals so that permissioned managers can pause withdrawals in response to major slashing events. Note that the manual pause can still be front-runned and does not fully mitigate the issue.

Assessed type

MEV

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Delegation/OperatorDelegator.sol#L265-L324 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Deposits/DepositQueue.sol#L134-L145

Vulnerability details

The fillERC20withdrawBuffer() function in the Deposit Queue can only be called by the Restake Manager.

function fillERC20withdrawBuffer(
    address _asset,
    uint256 _amount
) external nonReentrant onlyRestakeManager {
    // ... snip ...
}

However, the Operator Delegator calls fillERC20withdrawBuffer() when it completes a queued withdrawal.

function completeQueuedWithdrawal(
    // ... snip ...
    for (uint256 i; i < tokens.length; ) {
        // ... snip ...
        // check if token is not Native ETH
        if (address(tokens[i]) != IS_NATIVE) {
            // ... snip ...
            if (bufferToFill > 0) {
                // ... snip ..

                // fill Withdraw Buffer via depositQueue
                restakeManager.depositQueue().fillERC20withdrawBuffer(
                    address(tokens[i]),
                    bufferToFill
                );
            }
  // ... snip ..
}

This call to fillERC20withdrawBuffer() will revert because only the Restake Manager can call it. This leads to completeQueuedWithdrawal() to revert constantly.

Impact

Once the withdraw buffer for an ERC20 collateral token needs filling, queued withdrawals can no longer be completed. All users can no longer withdraw that collateral token until the withdraw buffer is refilled. The only way to fill the withdraw buffer is through deposits of the collateral token. However, since there is this issue with withdrawals, there is a great disincentive for users to deposit their ERC20s into the Restake Manager. This can lead to the deposited ERC20 tokens being locked in the protocol indefinitely unless a change to the code is made.

Proof of Concept

Below are the steps to reproduce the issue:

1. Alice deposits ERC20 tokens into the Restake Manager.
2. The Native ETH Restake Admin queues withdrawals in the Operator Delegator.
3. Alice withdraws her tokens right before the Restake Admin completes the queued withdrawals. This leads to a deficit in the withdraw buffer that needs filling and the issue gets triggered. The call to complete the queued withdrawals will revert and they can not be completed until the withdraw buffer is refilled via deposits.

Tools Used

Foundry, Manual Review

Recommended Mitigation Steps

Consider removing the access restriction in completeQueuedWithdrawal() or modify it to allow calls from all Operator Delegators.

Assessed type

Access Control

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/RestakeManager.sol#L316-L321 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/RestakeManager.sol#L500-L504 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/RestakeManager.sol#L594-L609 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/RestakeManager.sol#L652 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Withdraw/WithdrawQueue.sol#L217-L224 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/RateProvider/BalancerRateProvider.sol#L31-L40

Vulnerability details

The Restake Manager calculates the total TVL in the protocol including the Withdrawal Queue's TVL.

function calculateTVLs() public view returns (uint256[][] memory, uint256[] memory, uint256) {
    // ... snip ...
    for (uint256 i = 0; i < odLength; ) {
        // ... snip ...
        for (uint256 j = 0; j < tokenLength; ) {    
            // ... snip ...
            if (!withdrawQueueTokenBalanceRecorded) {
                totalWithdrawalQueueValue += renzoOracle.lookupTokenValue(
                    collateralTokens[i],
                    collateralTokens[j].balanceOf(withdrawQueue)
                );
            }
    // ... snip ...

There is a mistake, however, in the token being passed to lookupTokenValue() when computing for the Withdrawal Queue's TVL. It uses the index i instead of the index j, which means the token value being looked up every time is of the same token. This leads to inaccurate TVL calculations for the Withdrawal Queue.

For example, there are 3 collateral tokens wbETH, mETH and stETH:

1. wbETh is the first collateral token in the collateralTokens array.
2. The calculation for the total token value in the Withdrawal Queue will be wbETH price * wbETH balance + wbETH price * mETH balance + wbETH price * stETH.
3. The following prices will bloat the Withdrawal Queue's TVL because wbETH is more expensive than the other tokens:

• wbETH - $3,339.50
• stETH - $3,008.83
• mETH - $3,100.56

Impact

Users observing this behavior can exploit it to mint more ezETH tokens and withdraw more collateral tokens at the expense of other users. This is essentially stealing other users' deposits. This can be done for the life of the protocol unless the issue is fixed.

Proof of Concept

calculateTVLs() is used in the deposit and withdraw functions to compute for the mint and redeem amounts.

• RestakeManager.deposit()
• RestakeManager.depositEth()
• WithdrawQueue.withdraw()

The TVL is inversely correlated with the mint amounts and it is correlated with the withdrawn amounts. To exploit this issue, consider the following scenario:

• Users deposit collateral when the Withdraw Queue's withdraw buffer is empty or close to it to maximize the amount of ezETH minted to them.
• Users withdraw collateral when the Withdraw Queue's withdraw buffer is full of the collateral tokens to maximize the amounts withdrawn.
• Multiple iterations of bloated withdrawals and mints will lead to losses for other users.

Tools Used

Manual Review, Foundry

Recommended Mitigation Steps

The fix is changing the index i to index j when doing the lookup for the token's collateral value.

            if (!withdrawQueueTokenBalanceRecorded) {
                totalWithdrawalQueueValue += renzoOracle.lookupTokenValue(
-                   collateralTokens[i],
+                   collateralTokens[j]
                    collateralTokens[j].balanceOf(withdrawQueue)
                );
            }

Assessed type

Loop

Lines of code

https://github.com/Layr-Labs/eigenlayer-contracts/blob/dev/src/contracts/pods/DelayedWithdrawalRouter.sol#L100-L105 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Delegation/OperatorDelegator.sol#L501-L525 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Delegation/OperatorDelegator.sol#L481-L493 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Delegation/OperatorDelegator.sol#L470-L474

Vulnerability details

The Restake Admin is refunded gas for running some critical operations in the Operator Delegator such as:

• queueing withdrawals
• completing queued withdrawals
• verifying withdrawal credentials
• withdrawals are verified and processed

Gas is refunded to the ETH Restake Admin only when the Operator Delegator receives ETH from a non-Eigenpod address in a transaction that is started by the ETH Restake Admin.

receive() external payable nonReentrant {
    // check if sender contract is EigenPod. forward full withdrawal eth received
    if (msg.sender == address(eigenPod)) {
        restakeManager.depositQueue().forwardFullWithdrawalETH{ value: msg.value }();
    } else {
        // considered as protocol reward
        uint256 gasRefunded = 0;
        uint256 remainingAmount = msg.value;
        if (adminGasSpentInWei[tx.origin] > 0) {
            gasRefunded = _refundGas();
            // update the remaining amount
            remainingAmount -= gasRefunded;
            // If no funds left, return
            if (remainingAmount == 0) {
                return;
            }
        }
        // Forward remaining balance to the deposit queue
        address destination = address(restakeManager.depositQueue());
        (bool success, ) = destination.call{ value: remainingAmount }("");
        if (!success) revert TransferFailed();

        emit RewardsForwarded(destination, remainingAmount);
    }
}

The only ETH Restake Admin function that transfers ETH directly to the Operator Delegator is completeQueuedWithdrawal. All the rest either do not transfer ETH or transfer ETH to Eigenlayer's Delayed Withdrawal Router. To get a refund, the ETH Restake Admin must claim the delayed withdrawal from the Delayed Withdrawal Router.

When refunding gas, the gas refund is taken from the contract's balance.

function _refundGas() internal returns (uint256) {
    uint256 gasRefund = address(this).balance >= adminGasSpentInWei[tx.origin]
        ? adminGasSpentInWei[tx.origin]
        : address(this).balance;
    bool success = payable(tx.origin).send(gasRefund);
    if (!success) revert TransferFailed();

    // reset gas spent by admin
    adminGasSpentInWei[tx.origin] -= gasRefund;

    emit GasRefunded(tx.origin, gasRefund);
    return gasRefund;
}

However, in the receive callback, the amount of gas refunded is expected to be less than or equal to the ETH value received or it will revert due to underflow.

uint256 remainingAmount = msg.value;
if (adminGasSpentInWei[tx.origin] > 0) {
    gasRefunded = _refundGas();
    // update the remaining amount
    remainingAmount -= gasRefunded;

As long as the Operator Delegator has ETH balance lying around, the Restake Admin can only get refunded gas if the delayed withdrawals the Restake Admin claims are more than or equal to the gas refund. Otherwise, the Restake Admin transaction will revert due to the underflow.

Since claimDelayedWithdrawals is a permissionless function, anyone can call it to prevent the Restake Admin from getting their gas refunds. It is also generally good to claim delayed withdrawals gradually to avoid spikes in TVL that could lead to MEV. This makes it more likely the Restake Admin is not refunded their gas expenses for running the critical admin operations in Operator Delegator.

Impact

This issue can naturally occur and anyone can trigger it. The Restake Admin can be prevented from getting their gas refunds and the Restake Admin claiming delayed withdrawals can be forced to revert.

Proof of Concept

Consider the following scenario:

1. The Restake Admin has racked up gas for refunding worth 0.05 ETH.
2. Adversary self-destructs a contract to send 1 wei ETH to the Operator Delegator target.
3. Adversary or other users front-run the Restake Admin in claiming delayed withdrawals for target Operator Delegator.
4. Any delayed withdrawals claimed by the Restake Admin that sends less than 0.05 ETH will revert due to underflow.
5. As long the Restake Admin runs the critical operations in the Operator Delegator, their gas refund tab will only keep increasing and the more difficult it will be to get refunds.

Tools Used

Manual Review, Foundry

Recommended Mitigation Steps

Consider applying the following change in the receive callback.

-uint256 remainingAmount = msg.value;
+uint256 remainingAmount = msg.value;
// this is origin because Eigenlayer functions will send ETH to this contract.
if (adminGasSpentInWei[tx.origin] > 0) {

Refunding gas when receiving ETH from the Eigenpod may also be worth it.

Assessed type

Other