Undelegating by operators could freeze funds into EigenLayer.

[c4-bot-10]

Lines of code

https://github.com/Layr-Labs/eigenlayer-contracts/blob/dev/src/contracts/core/DelegationManager.sol#L211-L258 https://github.com/code-423n4/2024-04-renzo/blob/main/contracts/Delegation/OperatorDelegator.sol#L265-L324

Vulnerability details

Impact

Undelegating could lead to a permanent loss of funds.

Proof of Concept

In EigenLayer, operators can undelegate a staker(in our case OperatorDelegator contract) by calling the DelegationManager.undelegate().

    function undelegate(address staker) external onlyWhenNotPaused(PAUSED_ENTER_WITHDRAWAL_QUEUE) returns (bytes32[] memory withdrawalRoots) {
        require(isDelegated(staker), "DelegationManager.undelegate: staker must be delegated to undelegate");
        require(!isOperator(staker), "DelegationManager.undelegate: operators cannot be undelegated");
        require(staker != address(0), "DelegationManager.undelegate: cannot undelegate zero address");
        address operator = delegatedTo[staker];
        require(
            msg.sender == staker ||
                msg.sender == operator ||
                msg.sender == _operatorDetails[operator].delegationApprover,
            "DelegationManager.undelegate: caller cannot undelegate staker"
        );

        // Gather strategies and shares to remove from staker/operator during undelegation
        // Undelegation removes ALL currently-active strategies and shares
        (IStrategy[] memory strategies, uint256[] memory shares) = getDelegatableShares(staker);

        // emit an event if this action was not initiated by the staker themselves
        if (msg.sender != staker) {
            emit StakerForceUndelegated(staker, operator);
        }

        // undelegate the staker
        emit StakerUndelegated(staker, operator);
        delegatedTo[staker] = address(0);

        // if no delegatable shares, return an empty array, and don't queue a withdrawal
        if (strategies.length == 0) {
            withdrawalRoots = new bytes32[](0);
        } else {
            withdrawalRoots = new bytes32[](strategies.length);
            for (uint256 i = 0; i < strategies.length; i++) {
                IStrategy[] memory singleStrategy = new IStrategy[](1);
                uint256[] memory singleShare = new uint256[](1);
                singleStrategy[0] = strategies[i];
                singleShare[0] = shares[i];

                withdrawalRoots[i] = _removeSharesAndQueueWithdrawal({
                    staker: staker,
                    operator: operator,
                    withdrawer: staker,
                    strategies: singleStrategy,
                    shares: singleShare
                });
            }
        }

        return withdrawalRoots;
    }

The protocol then needs to withdraw the assets, but it could be impossible.

For withdrawing, it needs to call OperatorDelegator.completeQueuedWithdrawal(). Let's consider L281. When completing queued withdrawals, the mapping queuedShares is decreased by withdrawn amounts like at L281. However, if the OperatorDelegator was undelegated by operators, total amount of tokens that should be withdrawn exceeds the value of queuedShares, since during undelegating, a new withdrawing that withdraws remaining assets is created and queued and it doesn't increase the queuedShares of the OperatorDelegator contract. So, integer underflows will be occured at L281 near the last completeQueuedWithdrawal() transaction. As a result, the protocol can't complete the queued withdrawal made by undelegating, leading to a permanent loss of funds.

    function completeQueuedWithdrawal(
        IDelegationManager.Withdrawal calldata withdrawal,
        IERC20[] calldata tokens,
        uint256 middlewareTimesIndex
    ) external nonReentrant onlyNativeEthRestakeAdmin {
        [...]

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

        [...]
    }

Tools Used

Manual review

Recommended Mitigation Steps

Recommend to track the mapping variable delegatedTo of DelegationManager contract for undelegation checking in the functions OperatorDelegator.getTokenBalanceFromStrategy() and OperatorDelegator.getStakedETHBalance(), and add a mechanism to fix problems when it occurs.

Assessed type

Other

[DadeKuma]

@howlbot accept