Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Delegation/OperatorDelegator.sol#L326-L335

Vulnerability details

Impact

An incorrect implementation of the getTokenBalanceFromStrategy function in the OperatorDelegator contract can result in inaccurate TVL (Total Value Locked) calculation, which opens up an opportunity for a sandwich attack.

Proof of Concept

The calculateTVLs function in the RestakeManager contract calculates TVL by calling the getTokenBalanceFromStrategy function for each operator delegator.

 function calculateTVLs() public view returns (uint256[][] memory, uint256[] memory, uint256) {
    //......
    uint256 operatorBalance = operatorDelegators[i].getTokenBalanceFromStrategy(
        collateralTokens[j]
    );
    //......
 }

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/RestakeManager.sol#L302

The getTokenBalanceFromStrategy function checks if queuedShares[address(this)] is zero. If zero, it only considers the underlying token amount from the amount of shares. If not, it considers both the underlying token amount from the amount of shares and queued withdrawal shares.

    /// @dev Gets the underlying token amount from the amount of shares + queued withdrawal shares
    function getTokenBalanceFromStrategy(IERC20 token) external view returns (uint256) {
        return
            queuedShares[address(this)] == 0
                ? tokenStrategyMapping[token].userUnderlyingView(address(this))
                : tokenStrategyMapping[token].userUnderlyingView(address(this)) +
                    tokenStrategyMapping[token].sharesToUnderlyingView(
                        queuedShares[address(token)]
                    );
    }

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Delegation/OperatorDelegator.sol#L326-L335

The issue arises from using queuedShares[address(this)], which is always zero. This should be replaced with queuedShares[token].

Sandwich Attack Opportunity:

The attacker deposits into the protocol when there is ERC20 withdrawal request (but not yet completed). In this case, TVL will not takes into account the queued withdrawal value.
Admin calls completeQueuedWithdrawal to complete the queued withdrawal. This causes that the queued ERC20 be trasnferred to WithdrawQueue to fill the buffer, and the remaining will be deposited into the strategy again. All in all, this leads to increase in the TVL, because in the calculation of TVL, both the ERC20 balance of WithdrawQueue and ERC20 deposited in the strategy are taken into account.

The attacker requests to withdraw by burning his ezETH balance. Since, TVL is now increased, each ezETH is worthier than before. So, the attacker gains by receiving higher underlying token than before.

/**
function queueWithdrawals(
    IERC20[] calldata tokens,
    uint256[] calldata tokenAmounts
) external nonReentrant onlyNativeEthRestakeAdmin returns (bytes32) {
    // record gas spent
    uint256 gasBefore = gasleft();
    if (tokens.length != tokenAmounts.length) revert MismatchedArrayLengths();
    IDelegationManager.QueuedWithdrawalParams[]
        memory queuedWithdrawalParams = new IDelegationManager.QueuedWithdrawalParams[](1);
    // set strategies legth for 0th index only
    queuedWithdrawalParams[0].strategies = new IStrategy[](tokens.length);
    queuedWithdrawalParams[0].shares = new uint256[](tokens.length);

    // Save the nonce before starting the withdrawal
    uint96 nonce = uint96(delegationManager.cumulativeWithdrawalsQueued(address(this)));

    for (uint256 i; i < tokens.length; ) {
        if (address(tokens[i]) == IS_NATIVE) {
            // set beaconChainEthStrategy for ETH
            queuedWithdrawalParams[0].strategies[i] = eigenPodManager.beaconChainETHStrategy();

            // set shares for ETH
            queuedWithdrawalParams[0].shares[i] = tokenAmounts[i];
        } else {
            if (address(tokenStrategyMapping[tokens[i]]) == address(0))
                revert InvalidZeroInput();

            // set the strategy of the token
            queuedWithdrawalParams[0].strategies[i] = tokenStrategyMapping[tokens[i]];

            // set the equivalent shares for tokenAmount
            queuedWithdrawalParams[0].shares[i] = tokenStrategyMapping[tokens[i]]
                .underlyingToSharesView(tokenAmounts[i]);
        }

        // set withdrawer as this contract address
        queuedWithdrawalParams[0].withdrawer = address(this);

        // track shares of tokens withdraw for TVL
        queuedShares[address(tokens[i])] += queuedWithdrawalParams[0].shares[i];
        unchecked {
            ++i;
        }
    }

    // queue withdrawal in EigenLayer
    bytes32 withdrawalRoot = delegationManager.queueWithdrawals(queuedWithdrawalParams)[0];
    // Emit the withdrawal started event
    emit WithdrawStarted(
        withdrawalRoot,
        address(this),
        delegateAddress,
        address(this),
        nonce,
        block.number,
        queuedWithdrawalParams[0].strategies,
        queuedWithdrawalParams[0].shares
    );

    // update the gas spent for RestakeAdmin
    _recordGas(gasBefore);

    return withdrawalRoot;
}

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Delegation/OperatorDelegator.sol#L186-L256

function completeQueuedWithdrawal(
    IDelegationManager.Withdrawal calldata withdrawal,
    IERC20[] calldata tokens,
    uint256 middlewareTimesIndex
) external nonReentrant onlyNativeEthRestakeAdmin {
    uint256 gasBefore = gasleft();
    if (tokens.length != withdrawal.strategies.length) revert MismatchedArrayLengths();

    // complete the queued withdrawal from EigenLayer with receiveAsToken set to true
    delegationManager.completeQueuedWithdrawal(withdrawal, tokens, middlewareTimesIndex, true);

    IWithdrawQueue withdrawQueue = restakeManager.depositQueue().withdrawQueue();
    for (uint256 i; i < tokens.length; ) {
        if (address(tokens[i]) == address(0)) revert InvalidZeroInput();

        // deduct queued shares for tracking TVL
        queuedShares[address(tokens[i])] -= withdrawal.shares[i];

        // check if token is not Native ETH
        if (address(tokens[i]) != IS_NATIVE) {
            // Check the withdraw buffer and fill if below buffer target
            uint256 bufferToFill = withdrawQueue.getBufferDeficit(address(tokens[i]));

            // get balance of this contract
            uint256 balanceOfToken = tokens[i].balanceOf(address(this));
            if (bufferToFill > 0) {
                bufferToFill = (balanceOfToken <= bufferToFill) ? balanceOfToken : bufferToFill;

                // update amount to send to the operator Delegator
                balanceOfToken -= bufferToFill;

                // safe Approve for depositQueue
                tokens[i].safeApprove(address(restakeManager.depositQueue()), bufferToFill);

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

            // Deposit remaining tokens back to eigenLayer
            if (balanceOfToken > 0) {
                _deposit(tokens[i], balanceOfToken);
            }
        }
        unchecked {
            ++i;
        }
    }

    // emits the Withdraw Completed event with withdrawalRoot
    emit WithdrawCompleted(
        delegationManager.calculateWithdrawalRoot(withdrawal),
        withdrawal.strategies,
        withdrawal.shares
    );
    // record current spent gas
    _recordGas(gasBefore);
}

https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Delegation/OperatorDelegator.sol#L258-L324

Tools Used

Recommended Mitigation Steps

The following change is needed:

    function getTokenBalanceFromStrategy(IERC20 token) external view returns (uint256) {
        return
            queuedShares[address(token)] == 0
                ? tokenStrategyMapping[token].userUnderlyingView(address(this))
                : tokenStrategyMapping[token].userUnderlyingView(address(this)) +
                    tokenStrategyMapping[token].sharesToUnderlyingView(
                        queuedShares[address(token)]
                    );
    }

Assessed type

Context

code-423n4 / 2024-04-renzo-findings

Incorrect TVL calculation because of wrong implementation of `getTokenBalanceFromStrategy` #309