Potential null-deref in remote WMI enumeration

[tannergooding]

Note

This was reported via the feedback tool and was hand ported to GitHub

Summary

System.Management provides managed access to Windows Management Instrumentation (WMI). A common usage pattern is:

1. Create a ManagementScope pointing to a local or remote WMI namespace.
2. Execute a query with ManagementObjectSearcher.Get().
3. Enumerate the returned ManagementObjectCollection.

The relevant query entry point is ManagementObjectSearcher.Get(), which executes the WMI query and returns a ManagementObjectCollection backed by an IEnumWbemClassObject COM enumerator:

public ManagementObjectCollection Get()
{
    Initialize();
    IEnumWbemClassObject ew = null;
    SecurityHandler securityHandler = scope.GetSecurityHandler();
    EnumerationOptions enumOptions = (EnumerationOptions)options.Clone();

    int status = (int)ManagementStatus.NoError;

    try
    {
        // ...
        status = scope.GetSecuredIWbemServicesHandler(scope.GetIWbemServices()).ExecQuery_(
        query.QueryLanguage,
        query.QueryString,
        enumOptions.Flags,
        enumOptions.GetContext(),
        ref ew);
    }
    // ...

    return new ManagementObjectCollection(scope, options, ew);
}

The returned collection is evaluated lazily. The actual WMI objects are only pulled later by ManagementObjectEnumerator.MoveNext():

public bool MoveNext()
{
    if (isDisposed)
        throw new ObjectDisposedException(name);

    if (atEndOfCollection)
        return false;

    cacheIndex++;

    if ((cachedCount - cacheIndex) == 0)
    {
        IWbemClassObject_DoNotMarshal[] tempArray =
            new IWbemClassObject_DoNotMarshal[collectionObject.options.BlockSize];

        int status = collectionObject.scope
            .GetSecuredIEnumWbemClassObjectHandler(enumWbem)
            .Next_(timeout, (uint)collectionObject.options.BlockSize, tempArray, ref cachedCount);

        if (status >= 0)
        {
            for (int i = 0; i < cachedCount; i++)
            {
                cachedObjects[i] = new IWbemClassObjectFreeThreaded(
                    Marshal.GetIUnknownForObject(tempArray[i]));
            }
        }

        if (status == (int)tag_WBEMSTATUS.WBEM_S_TIMEDOUT && cachedCount == 0)
            ManagementException.ThrowWithExtendedInfo((ManagementStatus)status);

        if (status == (int)tag_WBEMSTATUS.WBEM_S_FALSE && cachedCount == 0)
        {
            atEndOfCollection = true;
            cacheIndex--;
            return false;
        }

        cacheIndex = 0;
    }
    return true;
}

this method can still return true even when no valid object has been written into cachedObjects[cacheIndex]. In other words, the code assumes that a non-negative return status implies a valid current object, but that assumption is not enforced before the cached object is later used.

Once Current is accessed, the cached entry is used without a null check:

public ManagementBaseObject Current
{
    get
    {
        if (isDisposed)
            throw new ObjectDisposedException(name);

        if (cacheIndex < 0)
            throw new InvalidOperationException();

        return ManagementBaseObject.GetBaseObject(
            cachedObjects[cacheIndex],
            collectionObject.scope);
    }
}

ManagementBaseObject.GetBaseObject() in turn immediately calls _IsClass():

internal static ManagementBaseObject GetBaseObject(
    IWbemClassObjectFreeThreaded wbemObject,
    ManagementScope scope)
{
    ManagementBaseObject newObject = null;

    if (_IsClass(wbemObject))
        newObject = ManagementClass.GetManagementClass(wbemObject, scope);
    else
        newObject = ManagementObject.GetManagementObject(wbemObject, scope);

    return newObject;
}

Finally, _IsClass() dereferences the object:

private static bool _IsClass(IWbemClassObjectFreeThreaded wbemObject)
{
    object val = null;
    int dummy1 = 0, dummy2 = 0;

    int status = wbemObject.Get_("__GENUS", 0, ref val, ref dummy1, ref dummy2);
    // ...
    return ((int)val == (int)tag_WBEM_GENUS_TYPE.WBEM_GENUS_CLASS);
}

If wbemObject is null at this point, the process terminates with an unhandled System.NullReferenceException.

An observed crash looks like this:

Unhandled exception. System.NullReferenceException: Object reference not set to an instance of an object.
at System.Management.ManagementBaseObject._IsClass(IWbemClassObjectFreeThreaded wbemObject)
at System.Management.ManagementBaseObject.GetBaseObject(IWbemClassObjectFreeThreaded wbemObject, ManagementScope scope)
at System.Management.ManagementObjectCollection.ManagementObjectEnumerator.get_Current()
at Program.Main(String[] args)

The managed code assumes that the native enumerator result is internally consistent and does not validate the cached object before use.

For comparison, ManagementEventWatcher contains a stricter guard and throws when cachedCount == 0 after a successful fetch path:

if (status >= 0)
{
    if (cachedCount == 0)
        ManagementException.ThrowWithExtendedInfo(ManagementStatus.Timedout);

    for (int i = 0; i < cachedCount; i++)
        cachedObjects[i] = new IWbemClassObjectFreeThreaded(
            Marshal.GetIUnknownForObject(tempArray[i]));
}

This suggests that the collection enumerator should also reject an empty successful batch instead of proceeding to dereference the cached entry.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-management
See info in area-owners.md if you want to be subscribed.

[tannergooding]

Given this requires a remote machine with a WMI endpoint that returns invalid data, it is difficult to reproduce. However, such endpoints can exist due to bugs or other problems and it would be goodness to ensure that we handle such potential issues appropriately.