[ros-diffs] [fireball] 54711: [NTOS] - A number of fixes to CmpQueryKeyName(): * Properly check the provided buffer size against needed size. * Don't overwrite user provided buffer. * Write as much data as cou...

[fireball]

Author: fireball
Date: Tue Dec 20 18:16:14 2011
New Revision: 54711

URL: http://svn.reactos.org/svn/reactos?rev=54711&view=rev
Log:
[NTOS]
- A number of fixes to CmpQueryKeyName():
 * Properly check the provided buffer size against needed size.
 * Don't overwrite user provided buffer.
 * Write as much data as could fit into the buffer (this is normal behaviour 
for any query function in the kernel), returning STATUS_INFO_LENGTH_MISMATCH if 
not all data were written.

Thanks to r3ddr4g0n for identifying the problem, testing with DPH and testing 
this patch.

Modified:
    trunk/reactos/ntoskrnl/config/cmsysini.c

Modified: trunk/reactos/ntoskrnl/config/cmsysini.c
URL: 
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/config/cmsysini.c?rev=54711&r1=54710&r2=54711&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/config/cmsysini.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/config/cmsysini.c [iso-8859-1] Tue Dec 20 18:16:14 
2011
@@ -121,6 +121,7 @@
                 IN KPROCESSOR_MODE PreviousMode)
 {
     PUNICODE_STRING KeyName;
+    ULONG BytesToCopy;
     NTSTATUS Status = STATUS_SUCCESS;
     PCM_KEY_BODY KeyBody = (PCM_KEY_BODY)ObjectBody;
     PCM_KEY_CONTROL_BLOCK Kcb = KeyBody->KeyControlBlock;
@@ -155,16 +156,32 @@
     /* Set the returned length */
     *ReturnLength = KeyName->Length + sizeof(OBJECT_NAME_INFORMATION) + 
sizeof(WCHAR);
 
-    /* Check if it fits into the provided buffer */
-    if ((Length < sizeof(OBJECT_NAME_INFORMATION)) ||
-        (Length < (*ReturnLength - sizeof(OBJECT_NAME_INFORMATION))))
+    /* Calculate amount of bytes to copy into the buffer */
+    BytesToCopy = KeyName->Length + sizeof(WCHAR);
+
+    /* Check if the provided buffer is too small to fit even anything */
+    if ((Length <= sizeof(OBJECT_NAME_INFORMATION)) ||
+        ((Length < (*ReturnLength)) && (BytesToCopy < sizeof(WCHAR))))
     {
         /* Free the buffer allocated by CmpConstructName */
         ExFreePool(KeyName);
 
-        /* Return buffer length failure */
+        /* Return buffer length failure without writing anything there because 
nothing fits */
         return STATUS_INFO_LENGTH_MISMATCH;
     }
+
+    /* Check if the provided buffer can be partially written */
+    if (Length < (*ReturnLength))
+    {
+        /* Yes, indicate so in the return status */
+        Status = STATUS_INFO_LENGTH_MISMATCH;
+
+        /* Calculate amount of bytes which the provided buffer could handle */
+        BytesToCopy = Length - sizeof(OBJECT_NAME_INFORMATION);
+    }
+
+    /* Remove the null termination character from the size */
+    BytesToCopy -= sizeof(WCHAR);
 
     /* Fill in the result */
     _SEH2_TRY
@@ -177,7 +194,10 @@
         /* Copy string content*/
         RtlCopyMemory(ObjectNameInfo->Name.Buffer,
                       KeyName->Buffer,
-                      *ReturnLength);
+                      BytesToCopy);
+
+        /* Null terminate it */
+        ObjectNameInfo->Name.Buffer[BytesToCopy / sizeof(WCHAR)] = 0;
     }
     _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER)
     {