[hpoussin] 31365: Fix hazardous UNICODE_STRING buffer manipulations

Author: hpoussin
Date: Fri Dec 21 12:40:32 2007
New Revision: 31365

URL: http://svn.reactos.org/svn/reactos?rev=31365&view=rev
Log:
Fix hazardous UNICODE_STRING buffer manipulations

Modified:
    trunk/reactos/ntoskrnl/io/pnpmgr/plugplay.c
    trunk/reactos/ntoskrnl/io/pnpmgr/pnproot.c

Modified: trunk/reactos/ntoskrnl/io/pnpmgr/plugplay.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/io/pnpmgr/plugplay...
==============================================================================
--- trunk/reactos/ntoskrnl/io/pnpmgr/plugplay.c (original)
+++ trunk/reactos/ntoskrnl/io/pnpmgr/plugplay.c Fri Dec 21 12:40:32 2007
@@ -43,7 +43,6 @@

return STATUS_SUCCESS;
 }
-

NTSTATUS
 IopQueueTargetDeviceEvent(const GUID *Guid,
@@ -258,119 +257,6 @@
 static PDEVICE_OBJECT
 IopGetDeviceObjectFromDeviceInstance(PUNICODE_STRING DeviceInstance)
 {
-#if 0
-    OBJECT_ATTRIBUTES ObjectAttributes;
-    UNICODE_STRING KeyName, ValueName;
-    LPWSTR KeyNameBuffer;
-    HANDLE InstanceKeyHandle;
-    HANDLE ControlKeyHandle;
-    NTSTATUS Status;
-    PKEY_VALUE_PARTIAL_INFORMATION ValueInformation;
-    ULONG ValueInformationLength;
-    PDEVICE_OBJECT DeviceObject = NULL;
-
-    DPRINT("IopGetDeviceObjectFromDeviceInstance(%wZ) called\n", DeviceInstance);
-
-    KeyNameBuffer = ExAllocatePool(PagedPool,
-                                   (49 * sizeof(WCHAR)) + DeviceInstance->Length);
-    if (KeyNameBuffer == NULL)
-    {
-        DPRINT1("Failed to allocate key name buffer!\n");
-        return NULL;
-    }
-
-    wcscpy(KeyNameBuffer, L"\Registry\Machine\System\CurrentControlSet\Enum\");
-    wcscat(KeyNameBuffer, DeviceInstance->Buffer);
-
-    RtlInitUnicodeString(&KeyName,
-                         KeyNameBuffer);
-    InitializeObjectAttributes(&ObjectAttributes,
-                               &KeyName,
-                               OBJ_CASE_INSENSITIVE,
-                               NULL,
-                               NULL);
-
-    Status = ZwOpenKey(&InstanceKeyHandle,
-                       KEY_READ,
-                       &ObjectAttributes);
-    ExFreePool(KeyNameBuffer);
-    if (!NT_SUCCESS(Status))
-    {
-        DPRINT1("Failed to open the instance key (Status %lx)\n", Status);
-        return NULL;
-    }
-
-    /* Open the 'Control' subkey */
-    RtlInitUnicodeString(&KeyName,
-                         L"Control");
-    InitializeObjectAttributes(&ObjectAttributes,
-                               &KeyName,
-                               OBJ_CASE_INSENSITIVE,
-                               InstanceKeyHandle,
-                               NULL);
-
-    Status = ZwOpenKey(&ControlKeyHandle,
-                       KEY_READ,
-                       &ObjectAttributes);
-    ZwClose(InstanceKeyHandle);
-    if (!NT_SUCCESS(Status))
-    {
-        DPRINT1("Failed to open the 'Control' key (Status %lx)\n", Status);
-        return NULL;
-    }
-
-    /* Query the 'DeviceReference' value */
-    RtlInitUnicodeString(&ValueName,
-                         L"DeviceReference");
-    ValueInformationLength = FIELD_OFFSET(KEY_VALUE_PARTIAL_INFORMATION,
-                             Data[0]) + sizeof(ULONG);
-    ValueInformation = ExAllocatePool(PagedPool, ValueInformationLength);
-    if (ValueInformation == NULL)
-    {
-        DPRINT1("Failed to allocate the name information buffer!\n");
-        ZwClose(ControlKeyHandle);
-        return NULL;
-    }
-
-    Status = ZwQueryValueKey(ControlKeyHandle,
-                             &ValueName,
-                             KeyValuePartialInformation,
-                             ValueInformation,
-                             ValueInformationLength,
-                             &ValueInformationLength);
-    ZwClose(ControlKeyHandle);
-    if (!NT_SUCCESS(Status))
-    {
-        DPRINT1("Failed to query the 'DeviceReference' value (Status %lx)\n", Status);
-        return NULL;
-    }
-
-    /* Check the device object */
-    RtlCopyMemory(&DeviceObject,
-                  ValueInformation->Data,
-                  sizeof(PDEVICE_OBJECT));
-
-    DPRINT("DeviceObject: %p\n", DeviceObject);
-
-    if (DeviceObject->Type != IO_TYPE_DEVICE ||
-        DeviceObject->DeviceObjectExtension == NULL ||
-        DeviceObject->DeviceObjectExtension->DeviceNode == NULL ||
-        !RtlEqualUnicodeString(&DeviceObject->DeviceObjectExtension->DeviceNode->InstancePath,
-                               DeviceInstance, TRUE))
-    {
-        DPRINT1("Invalid object type!\n");
-        return NULL;
-    }
-
-    DPRINT("Instance path: %wZ\n", &DeviceObject->DeviceObjectExtension->DeviceNode->InstancePath);
-
-    ObReferenceObject(DeviceObject);
-
-    DPRINT("IopGetDeviceObjectFromDeviceInstance() done\n");
-
-    return DeviceObject;
-#endif
-
     if (IopRootDeviceNode == NULL)
         return NULL;

Modified: trunk/reactos/ntoskrnl/io/pnpmgr/pnproot.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/io/pnpmgr/pnproot....
==============================================================================
--- trunk/reactos/ntoskrnl/io/pnpmgr/pnproot.c (original)
+++ trunk/reactos/ntoskrnl/io/pnpmgr/pnproot.c Fri Dec 21 12:40:32 2007
@@ -83,7 +83,7 @@

typedef struct _BUFFER
 {
-    PVOID *Buffer;
+    PVOID *Data;
     PULONG Length;
 } BUFFER, *PBUFFER;

@@ -235,19 +235,17 @@
     IN PVOID Context,
     IN PVOID EntryContext)
 {
-    PUNICODE_STRING String = (PUNICODE_STRING)EntryContext;
+    PUNICODE_STRING Destination = (PUNICODE_STRING)EntryContext;
+    UNICODE_STRING Source;

if (ValueType != REG_SZ || ValueLength == 0 || ValueLength % sizeof(WCHAR) != 0)
         return STATUS_SUCCESS;

-    String->Buffer = ExAllocatePoolWithTag(PagedPool, ValueLength, TAG_PNP_ROOT);
-    if (String->Buffer == NULL)
-        return STATUS_NO_MEMORY;
-    String->Length = String->MaximumLength = (USHORT)ValueLength;
-    RtlCopyMemory(String->Buffer, ValueData, ValueLength);
-    if (ValueLength > 0 && String->Buffer[ValueLength / sizeof(WCHAR) - 1] == L'\0')
-        String->Length -= sizeof(WCHAR);
-    return STATUS_SUCCESS;
+    Source.MaximumLength = Source.Length = ValueLength;
+    Source.Buffer = ValueData;
+    if (Source.Length > 0 && Source.Buffer[Source.Length / sizeof(WCHAR) - 1] == UNICODE_NULL)
+        Source.Length -= sizeof(WCHAR);
+    return RtlDuplicateUnicodeString(RTL_DUPLICATE_UNICODE_STRING_NULL_TERMINATE, &Source, Destination);
 }

static NTSTATUS NTAPI
@@ -264,7 +262,7 @@

if (ValueLength == 0)
     {
-        *Buffer->Buffer = NULL;
+        *Buffer->Data = NULL;
         return STATUS_SUCCESS;
     }

@@ -272,7 +270,7 @@
     if (BinaryValue == NULL)
         return STATUS_NO_MEMORY;
     RtlCopyMemory(BinaryValue, ValueData, ValueLength);
-    *Buffer->Buffer = BinaryValue;
+    *Buffer->Data = BinaryValue;
     if (Buffer->Length) *Buffer->Length = ValueLength;
     return STATUS_SUCCESS;
 }
@@ -454,9 +452,9 @@
                 }

/* Fill other informations */
-                Buffer1.Buffer = (PVOID *)&Device->ResourceRequirementsList;
+                Buffer1.Data = (PVOID *)&Device->ResourceRequirementsList;
                 Buffer1.Length = NULL;
-                Buffer2.Buffer = (PVOID *)&Device->ResourceList;
+                Buffer2.Data = (PVOID *)&Device->ResourceList;
                 Buffer2.Length = &Device->ResourceListSize;
                 RtlZeroMemory(QueryTable, sizeof(QueryTable));
                 QueryTable[0].QueryRoutine = QueryStringCallback;