Potential null-deref in remote WMI enumeration
- Dominant language
- C#
- Stars
- 18.3k
- Forks
- 5.6k
- PR merge metrics
- PR metrics pending
Description
> [!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:
```csharp
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()`:
```csharp
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:
```csharp
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()`:
```csharp
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:
```csharp
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:
```text
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:
```csharp
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.
Contributor guide
Assessment
This issue has not been assessed yet.