[ros-diffs] [fireball] 31565: - Get rid of possible buffer overflows and memory corruptions due to an assumption that UNICODE_STRING's buffers are always NULL-terminated.

Author: fireball
Date: Wed Jan  2 19:06:03 2008
New Revision: 31565

URL: http://svn.reactos.org/svn/reactos?rev=31565&view=rev
Log:
- Get rid of possible buffer overflows and memory corruptions due to an assumption that
UNICODE_STRING's buffers are always NULL-terminated.

Modified:
    trunk/reactos/ntoskrnl/io/iomgr/driver.c
    trunk/reactos/ntoskrnl/io/pnpmgr/pnpmgr.c

Modified: trunk/reactos/ntoskrnl/io/iomgr/driver.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/io/iomgr/driver.c…
==============================================================================
--- trunk/reactos/ntoskrnl/io/iomgr/driver.c (original)
+++ trunk/reactos/ntoskrnl/io/iomgr/driver.c Wed Jan  2 19:06:03 2008
@@ -222,27 +222,31 @@
 
    if (InputImagePath.Length == 0)
    {
-      ImagePath->Length = (33 * sizeof(WCHAR)) + ServiceName->Length;
-      ImagePath->MaximumLength = ImagePath->Length + sizeof(UNICODE_NULL);
+      ImagePath->Length = 0;
+      ImagePath->MaximumLength =
+          (33 * sizeof(WCHAR)) + ServiceName->Length + sizeof(UNICODE_NULL);
       ImagePath->Buffer = ExAllocatePool(NonPagedPool, ImagePath->MaximumLength);
       if (ImagePath->Buffer == NULL)
          return STATUS_NO_MEMORY;
 
-      wcscpy(ImagePath->Buffer, L"\\SystemRoot\\system32\\drivers\\");
-      wcscat(ImagePath->Buffer, ServiceName->Buffer);
-      wcscat(ImagePath->Buffer, L".sys");
+      RtlAppendUnicodeToString(ImagePath,
L"\\SystemRoot\\system32\\drivers\\");
+      RtlAppendUnicodeStringToString(ImagePath, ServiceName);
+      RtlAppendUnicodeToString(ImagePath, L".sys");
    } else
    if (InputImagePath.Buffer[0] != L'\\')
    {
-      ImagePath->Length = (12 * sizeof(WCHAR)) + InputImagePath.Length;
-      ImagePath->MaximumLength = ImagePath->Length + sizeof(UNICODE_NULL);
+      ImagePath->Length = 0;
+      ImagePath->MaximumLength =
+          12 * sizeof(WCHAR) + InputImagePath.Length + sizeof(UNICODE_NULL);
       ImagePath->Buffer = ExAllocatePool(NonPagedPool, ImagePath->MaximumLength);
       if (ImagePath->Buffer == NULL)
          return STATUS_NO_MEMORY;
 
-      wcscpy(ImagePath->Buffer, L"\\SystemRoot\\");
-      wcscat(ImagePath->Buffer, InputImagePath.Buffer);
-      ExFreePool(InputImagePath.Buffer);
+      RtlAppendUnicodeToString(ImagePath, L"\\SystemRoot\\");
+      RtlAppendUnicodeStringToString(ImagePath, &InputImagePath);
+
+      /* Free caller's string */
+      RtlFreeUnicodeString(&InputImagePath);
    }
 
    return STATUS_SUCCESS;

Modified: trunk/reactos/ntoskrnl/io/pnpmgr/pnpmgr.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/io/pnpmgr/pnpmgr.…
==============================================================================
--- trunk/reactos/ntoskrnl/io/pnpmgr/pnpmgr.c (original)
+++ trunk/reactos/ntoskrnl/io/pnpmgr/pnpmgr.c Wed Jan  2 19:06:03 2008
@@ -1593,7 +1593,6 @@
                      PUNICODE_STRING ParentIdPrefix)
 {
    ULONG KeyNameBufferLength;
-   PWSTR KeyNameBuffer = NULL;
    PKEY_VALUE_PARTIAL_INFORMATION ParentIdPrefixInformation = NULL;
    UNICODE_STRING KeyName;
    UNICODE_STRING KeyValue;
@@ -1620,15 +1619,20 @@
        Status = STATUS_INSUFFICIENT_RESOURCES;
        goto cleanup;
    }
-   KeyNameBuffer = ExAllocatePool(PagedPool, (49 * sizeof(WCHAR)) +
DeviceNode->Parent->InstancePath.Length);
-   if (!KeyNameBuffer)
+
+
+   KeyName.Buffer = ExAllocatePool(PagedPool, (49 * sizeof(WCHAR)) +
DeviceNode->Parent->InstancePath.Length);
+   if (!KeyName.Buffer)
    {
        Status = STATUS_INSUFFICIENT_RESOURCES;
        goto cleanup;
    }
-   wcscpy(KeyNameBuffer,
L"\\Registry\\Machine\\System\\CurrentControlSet\\Enum\\");
-   wcscat(KeyNameBuffer, DeviceNode->Parent->InstancePath.Buffer);
-   RtlInitUnicodeString(&KeyName, KeyNameBuffer);
+   KeyName.Length = 0;
+   KeyName.MaximumLength = (49 * sizeof(WCHAR)) +
DeviceNode->Parent->InstancePath.Length;
+
+   RtlAppendUnicodeToString(&KeyName,
L"\\Registry\\Machine\\System\\CurrentControlSet\\Enum\\");
+   RtlAppendUnicodeStringToString(&KeyName,
&DeviceNode->Parent->InstancePath);
+
    Status = IopOpenRegistryKeyEx(&hKey, NULL, &KeyName, KEY_QUERY_VALUE |
KEY_SET_VALUE);
    if (!NT_SUCCESS(Status))
       goto cleanup;
@@ -1678,7 +1682,7 @@
       Status = RtlDuplicateUnicodeString(RTL_DUPLICATE_UNICODE_STRING_NULL_TERMINATE,
&KeyValue, ParentIdPrefix);
    }
    ExFreePool(ParentIdPrefixInformation);
-   ExFreePool(KeyNameBuffer);
+   RtlFreeUnicodeString(&KeyName);
    if (hKey != NULL)
       ZwClose(hKey);
    return Status;
@@ -1713,7 +1717,6 @@
    WCHAR InstancePath[MAX_PATH];
    IO_STACK_LOCATION Stack;
    NTSTATUS Status;
-   PWSTR KeyBuffer;
    PWSTR Ptr;
    USHORT Length;
    USHORT TotalLength;
@@ -1851,18 +1854,11 @@
    /*
     * Create registry key for the instance id, if it doesn't exist yet
     */
-   KeyBuffer = ExAllocatePool(
-      PagedPool,
-      (49 * sizeof(WCHAR)) + DeviceNode->InstancePath.Length);
-   wcscpy(KeyBuffer,
L"\\Registry\\Machine\\System\\CurrentControlSet\\Enum\\");
-   wcscat(KeyBuffer, DeviceNode->InstancePath.Buffer);
-   Status = IopCreateDeviceKeyPath(/*KeyBuffer*/&DeviceNode->InstancePath,
&InstanceKey);
-   ExFreePool(KeyBuffer);
+   Status = IopCreateDeviceKeyPath(&DeviceNode->InstancePath, &InstanceKey);
    if (!NT_SUCCESS(Status))
    {
       DPRINT1("Failed to create the instance key! (Status %lx)\n", Status);
    }
-
 
    {
       /* Set 'Capabilities' value */