[ros-diffs] [reactos] 01/01: [NTOS:OB] Properly calculate the return length in ObQueryTypeInfo

[George Bișoc]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=00c69bcd15e44ee961f4d…

commit 00c69bcd15e44ee961f4dfa7503cc0012c4beadd
Author:     George Bișoc <george.bisoc(a)reactos.org>
AuthorDate: Sat Apr 15 12:55:32 2023 +0200
Commit:     George Bișoc <george.bisoc(a)reactos.org>
CommitDate: Fri Apr 21 12:45:31 2023 +0200

    [NTOS:OB] Properly calculate the return length in ObQueryTypeInfo
    
    On a x86 system aligning the return length pointer to a 4-byte boundary
    works best since pointers in general are 4-byte aligned for x86 systems.
    
    However, what happens on a AMD64 system is that we still align this pointer
    to 4-byte, ObjectTypeInfo is a 8-byte pointer and we might write into
    the return length past the 4-byte boundary.
    
    If one were to allocate a pool of memory with that length and query all
    the object types info and free the said pool of memory thereafter, the
    system will crash with BAD_POOL_HEADER because ObQueryTypeInfo overwrote
    the return length past the 4-byte boundary length therefore leading up with
    corrupted memory blocks in the pool header.
    
    This symptom of BAD_POOL_HEADER happens exactly the same in Windows Server
    2003 x64 Edition. Newer versions of Windows like 10 aren't affected.
    
    But, Windows has another bug where they are using MaximumLength for the
    calculation of the needed length to be returned to caller. MaximumLength
    does not guarantee you that it includes the NULL-terminator in the length
    and that potentially leads to a buffer overrun.
    
    Also annotate the ObQueryTypeInfo function with SAL2.
    
    https://processhacker.sourceforge.io/doc/object_8c_source.html (read the
    comment in KphObjectTypeInformation).
---
 ntoskrnl/ob/oblife.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/ntoskrnl/ob/oblife.c b/ntoskrnl/ob/oblife.c
index a6161a4bbf7..f0ae6aafee3 100644
--- a/ntoskrnl/ob/oblife.c
+++ b/ntoskrnl/ob/oblife.c
@@ -944,22 +944,36 @@ ObpQueryNameInfoSize(
 
 NTSTATUS
 NTAPI
-ObQueryTypeInfo(IN POBJECT_TYPE ObjectType,
-                OUT POBJECT_TYPE_INFORMATION ObjectTypeInfo,
-                IN ULONG Length,
-                OUT PULONG ReturnLength)
+ObQueryTypeInfo(
+    _In_ POBJECT_TYPE ObjectType,
+    _Out_writes_bytes_(Length) POBJECT_TYPE_INFORMATION ObjectTypeInfo,
+    _In_ ULONG Length,
+    _Out_ PULONG ReturnLength)
 {
     NTSTATUS Status = STATUS_SUCCESS;
     PWSTR InfoBuffer;
 
+    /* The string of the object type name has to be NULL-terminated */
+    ASSERT(ObjectType->Name.MaximumLength >= ObjectType->Name.Length +
sizeof(UNICODE_NULL));
+
     /* Enter SEH */
     _SEH2_TRY
     {
-        /* Set return length aligned to 4-byte boundary */
+        /*
+         * Set return length aligned to 4-byte or 8-byte boundary. Windows has a bug
+         * where the returned length pointer is always aligned to a 4-byte boundary.
+         * If one were to allocate a pool of memory in kernel mode to retrieve all
+         * the object types info with this return length, Windows will bugcheck with
+         * BAD_POOL_HEADER in 64-bit upon you free the said allocated memory.
+         *
+         * More than that, Windows uses MaximumLength for the calculation of the
returned
+         * length and MaximumLength does not always guarantee the name type is
NULL-terminated
+         * leading the ObQueryTypeInfo function to overrun the buffer.
+         */
         *ReturnLength += sizeof(*ObjectTypeInfo) +
-                         ALIGN_UP(ObjectType->Name.MaximumLength, ULONG);
+                         ALIGN_UP(ObjectType->Name.Length + sizeof(UNICODE_NULL),
ULONG_PTR);
 
-        /* Check if thats too much though. */
+        /* Check if that is too much */
         if (Length < *ReturnLength)
         {
             _SEH2_YIELD(return STATUS_INFO_LENGTH_MISMATCH);