[NtTypes] Correctly compute buffer size allocated for ObjectTypesInformation

[1orenz0]

ntdll.NtQueryObject is one in a long list of natives API with a pretty
funny way to use it. It takes a buffer and a buffer size, and on success
fills out the bufffer and write the written length in a return_length
parameter.

 using (var type_info = new SafeStructureInOutBuffer<ObjectAllTypesInformation>())
{
  Dictionary<string, NtType> ret = new Dictionary<string, NtType>(StringComparer.OrdinalIgnoreCase);
  NtStatus status = NtSystemCalls.NtQueryObject(
    SafeKernelObjectHandle.Null, 
    ObjectInformationClass.ObjectTypesInformation,
    type_info, 
    type_info.Length, 
    out int return_length
  );

  if (status != NtStatus.STATUS_INFO_LENGTH_MISMATCH)
    status.ToNtException();

  return return_length;
}

However if the buffer is not big enough, the return status is set to
STATUS_INFO_LENGTH_MISMATCH inciting the developer to embiggen the input
buffer and re-attempt the call. However return_length parameter is also
sometimes set but it should not be relied upon as long as status is not
equal to STATUS_SUCCESS.

It's use to enumerate Object types in NtType.LoadTypes. I've added debug log to understand the situation :

private static Dictionary<string, NtType> LoadTypes()
  {
    var type_factories = NtTypeFactory.GetAssemblyNtTypeFactories(Assembly.GetExecutingAssembly());
    int type_size = GetTypeSize();
    Dictionary<string, NtType> ret = new Dictionary<string, NtType>(StringComparer.OrdinalIgnoreCase);
    Console.WriteLine("type_size {0:x}", type_size);
    using (var type_info = new SafeStructureInOutBuffer<ObjectAllTypesInformation>(type_size, true))
    {
      int alignment = IntPtr.Size - 1;
      NtSystemCalls.NtQueryObject(SafeKernelObjectHandle.Null, ObjectInformationClass.ObjectTypesInformation,
                    type_info, type_info.Length, out int return_length).ToNtException();
      Console.WriteLine("return_length {0:x}", return_length);
      ObjectAllTypesInformation result = type_info.Result;
      IntPtr curr_typeinfo = type_info.DangerousGetHandle() + IntPtr.Size;
      ...
      Console.WriteLine("done");

On a 64-bit build :

type_size 2278
return_length 2278
done

On a 32-bit build :

type_size 1fd8
return_length 1fe4
done

We have a classic buffer overflow since the return_length value returned on a STATUS_INFO_LENGTH_MISMATCH is not quite the correct value (why is that, I honestly don't know or/and care).

The fix is inspired by how ProcessHacker use the API :

NTSTATUS PhEnumObjectTypes(
    _Out_ POBJECT_TYPES_INFORMATION *ObjectTypes
    )
{
    NTSTATUS status;
    PVOID buffer;
    ULONG bufferSize;
    ULONG returnLength;

    bufferSize = 0x1000;
    buffer = PhAllocate(bufferSize);

    while ((status = NtQueryObject(
        NULL,
        ObjectTypesInformation,
        buffer,
        bufferSize,
        &returnLength
        )) == STATUS_INFO_LENGTH_MISMATCH)
    {
        PhFree(buffer);
        bufferSize *= 2;

        // Fail if we're resizing the buffer to something very large.
        if (bufferSize > PH_LARGE_BUFFER_SIZE)
            return STATUS_INSUFFICIENT_RESOURCES;

        buffer = PhAllocate(bufferSize);
    }

    if (!NT_SUCCESS(status))
    {
        PhFree(buffer);
        return status;
    }

    *ObjectTypes = (POBJECT_TYPES_INFORMATION)buffer;

    return status;
}

[googlebot]

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

[tyranid]

Hmm, wow, okay. Wouldn't surprise me if there's a bug in the wow64 layer (there's a few of those) as regardless of whether the return length value is correct or not (seems fine on 64bit) it shouldn't end up overwriting the end of the allocated size we passed in to begin with. I have noticed there seemed to be a memory corruption somewhere in 32 bit, wonder if this is it?

Can you fix the CLA issue? Also I have at least one comment on the commit to resolve.

[tyranid]

Can we just roll this all into LoadTypes()? Seems unnecessary to calculate the length in a loop then actually do then just return that size and drop all the type information we gathered just to do it again. Perhaps cap at max of around 1MiB for buffer size and start with size of say 8192 bytes as for all current versions of Windows that should result in never having to loop at all.

[1orenz0]

yeah I kept every changes in GetTypeSize() in order for the PR to be simpler, but it makes every sense to move it back to LoadTypes(). Since only LoadTypes calls GetTypeSize, we might as well get rid of GetTypeSize.

[tyranid]

Don't really need a comment on an private method.

[tyranid]

As said perhaps cap at around 1MiB as I can't imagine it ever going close to that sort of size. If we can extract type information something more serious is going wrong.

[googlebot]

CLAs look good, thanks!

[tyranid]

Best replace this with a "throw new NtException(status);" as ToNtException only throws on error codes but we want to throw for everything.

[tyranid]

I'd replace the while loop with a check against the size (i.e. size < 0x100000) then move the definition of status of the dictionary into the loop. Then you can replace the redundant return with a "throw new NtException(NtStatus.STATUS_INSUFFICIENT_RESOURCES)".

[1orenz0]

got it.

[tyranid]

Thanks a lot, really appreciate the fix.

[alexey-monastyrsky]

NtQueryObject also sometimes returns STATUS_INFO_LENGTH_MISMATCH if the provided buffer is bigger than expected. At least this the case on my Windows 10 x64 RS4 when using ObjectBasicInformation info class.

This code passes a buffer of exactly the required size and gets STATUS_SUCCESS in return:

using (var objectInfoBuf = new SafeStructureInOutBuffer<ObjectBasicInformation>(0, true))
{
	var status = NtSystemCalls.NtQueryObject(
		NtProcess.Current.Handle, 
		ObjectInformationClass.ObjectBasicInformation, 
		objectInfoBuf, objectInfoBuf.Length, 
		out int returnLength);
}

But if I increase the buffer size by 1, the call results in STATUS_INFO_LENGTH_MISMATCH (and no correct value in returnLength):

using (var objectInfoBuf = new SafeStructureInOutBuffer<ObjectBasicInformation>(1, true))
{
	var status = NtSystemCalls.NtQueryObject(
		NtProcess.Current.Handle, 
		ObjectInformationClass.ObjectBasicInformation, 
		objectInfoBuf, objectInfoBuf.Length, 
		out int returnLength);
}