[janderwald] 33088: - check input in PNP_GetRootDeviceInstance - fix length check in PNP_GetClassName - allocate device id string dynamically in to avoid a potential buffer overflow - use lstrlenW over wcslen in PnpEventThread

Author: janderwald
Date: Mon Apr 21 07:08:42 2008
New Revision: 33088

URL: http://svn.reactos.org/svn/reactos?rev=33088&view=rev
Log:
- check input in PNP_GetRootDeviceInstance
- fix length check in PNP_GetClassName
- allocate device id string dynamically in to avoid a potential buffer overflow
- use lstrlenW over wcslen in PnpEventThread

Modified:
    trunk/reactos/base/services/umpnpmgr/umpnpmgr.c

Modified: trunk/reactos/base/services/umpnpmgr/umpnpmgr.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/base/services/umpnpmgr/ump…
==============================================================================
--- trunk/reactos/base/services/umpnpmgr/umpnpmgr.c [iso-8859-1] (original)
+++ trunk/reactos/base/services/umpnpmgr/umpnpmgr.c [iso-8859-1] Mon Apr 21 07:08:42 2008
@@ -308,6 +308,11 @@
 
     DPRINT("PNP_GetRootDeviceInstance() called\n");
 
+    if (!pDeviceID)
+    {
+        ret = CR_INVALID_POINTER;
+        goto Done;
+    }
     if (ulLength < lstrlenW(szRootDeviceId) + 1)
     {
         ret = CR_BUFFER_SMALL;
@@ -891,9 +896,8 @@
 
     DPRINT("PNP_GetClassName() called\n");
 
-    lstrcpyW(szKeyName, L"System\\CurrentControlSet\\Control\\Class");
-    lstrcatW(szKeyName, L"\\");
-    if(lstrlenW(pszClassGuid) < sizeof(szKeyName)/sizeof(WCHAR)-lstrlenW(szKeyName))
+    lstrcpyW(szKeyName, L"System\\CurrentControlSet\\Control\\Class\\");
+    if(lstrlenW(pszClassGuid) + 1 <
sizeof(szKeyName)/sizeof(WCHAR)-(lstrlenW(szKeyName) * sizeof(WCHAR)))
         lstrcatW(szKeyName, pszClassGuid);
     else return CR_INVALID_DATA;
 
@@ -1250,7 +1254,8 @@
     HKEY hDeviceKey;
     LPWSTR pszSubKey;
     DWORD dwDeviceIdListSize;
-    WCHAR szDeviceIdList[512];
+    DWORD dwNewDeviceIdSize;
+    WCHAR * pszDeviceIdList = NULL;
 
     UNREFERENCED_PARAMETER(hBinding);
 
@@ -1271,12 +1276,11 @@
 
     pszSubKey = (ulFlags & CM_ADD_ID_COMPATIBLE) ? L"CompatibleIDs" :
L"HardwareID";
 
-    dwDeviceIdListSize = 512 * sizeof(WCHAR);
     if (RegQueryValueExW(hDeviceKey,
                          pszSubKey,
                          NULL,
                          NULL,
-                         (LPBYTE)szDeviceIdList,
+                         NULL,
                          &dwDeviceIdListSize) != ERROR_SUCCESS)
     {
         DPRINT("Failed to query the desired ID string!\n");
@@ -1284,8 +1288,37 @@
         goto Done;
     }
 
+    dwNewDeviceIdSize = lstrlenW(pszDeviceID);
+    if (!dwNewDeviceIdSize)
+    {
+        ret = CR_INVALID_POINTER;
+        goto Done;
+    }
+
+    dwDeviceIdListSize += (dwNewDeviceIdSize + 2) * sizeof(WCHAR);
+
+    pszDeviceIdList = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, dwDeviceIdListSize);
+    if (!pszDeviceIdList)
+    {
+        DPRINT("Failed to allocate memory for the desired ID string!\n");
+        ret = CR_OUT_OF_MEMORY;
+        goto Done;
+    }
+
+    if (RegQueryValueExW(hDeviceKey,
+                         pszSubKey,
+                         NULL,
+                         NULL,
+                         (LPBYTE)pszDeviceIdList,
+                         &dwDeviceIdListSize) != ERROR_SUCCESS)
+    {
+        DPRINT("Failed to query the desired ID string!\n");
+        ret = CR_REGISTRY_ERROR;
+        goto Done;
+    }
+
     /* Check whether the device ID is already in use */
-    if (CheckForDeviceId(szDeviceIdList, pszDeviceID))
+    if (CheckForDeviceId(pszDeviceIdList, pszDeviceID))
     {
         DPRINT("Device ID was found in the ID string!\n");
         ret = CR_SUCCESS;
@@ -1293,13 +1326,13 @@
     }
 
     /* Append the Device ID */
-    AppendDeviceId(szDeviceIdList, &dwDeviceIdListSize, pszID);
+    AppendDeviceId(pszDeviceIdList, &dwDeviceIdListSize, pszID);
 
     if (RegSetValueExW(hDeviceKey,
                        pszSubKey,
                        0,
                        REG_MULTI_SZ,
-                       (LPBYTE)szDeviceIdList,
+                       (LPBYTE)pszDeviceIdList,
                        dwDeviceIdListSize) != ERROR_SUCCESS)
     {
         DPRINT("Failed to set the desired ID string!\n");
@@ -1308,6 +1341,8 @@
 
 Done:
     RegCloseKey(hDeviceKey);
+    if (pszDeviceIdList)
+        HeapFree(GetProcessHeap(), 0, pszDeviceIdList);
 
     DPRINT("PNP_AddID() done (returns %lx)\n", ret);
 
@@ -2071,22 +2106,26 @@
         {
             DeviceInstallParams* Params;
             DWORD len;
+            DWORD DeviceIdLength;
 
             DPRINT("Device arrival event: %S\n",
PnpEvent->TargetDevice.DeviceIds);
 
-            /* Queue device install (will be dequeued by DeviceInstallThread */
-            len = FIELD_OFFSET(DeviceInstallParams, DeviceIds)
-                + wcslen(PnpEvent->TargetDevice.DeviceIds) * sizeof(WCHAR) +
sizeof(UNICODE_NULL);
-            Params = HeapAlloc(GetProcessHeap(), 0, len);
-            if (Params)
+            DeviceIdLength = lstrlenW(PnpEvent->TargetDevice.DeviceIds);
+            if (DeviceIdLength)
             {
-                wcscpy(Params->DeviceIds, PnpEvent->TargetDevice.DeviceIds);
+                /* Queue device install (will be dequeued by DeviceInstallThread */
+                len = FIELD_OFFSET(DeviceInstallParams, DeviceIds) + (DeviceIdLength + 1)
* sizeof(WCHAR);
+                Params = HeapAlloc(GetProcessHeap(), 0, len);
+                if (Params)
+                {
+                    wcscpy(Params->DeviceIds, PnpEvent->TargetDevice.DeviceIds);
 #ifdef HAVE_SLIST_ENTRY_IMPLEMENTED
-                InterlockedPushEntrySList(&DeviceInstallListHead,
&Params->ListEntry);
+                    InterlockedPushEntrySList(&DeviceInstallListHead,
&Params->ListEntry);
 #else
-                InsertTailList(&DeviceInstallListHead, &Params->ListEntry);
+                    InsertTailList(&DeviceInstallListHead,
&Params->ListEntry);
 #endif
-                SetEvent(hDeviceInstallListNotEmpty);
+                    SetEvent(hDeviceInstallListNotEmpty);
+                }
             }
         }
         else