[PATCH 0/1] MR11287: cfgmgr32: Fix CM_Get_Device_ID buffer size handling.

[Rémi Bernon (＠rbernon)]

CM_Get_Device_ID_ExW() reserved no room for the terminating null in its
size check, which might overwrite outside the buffer bounds.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/11287

[PATCH 1/1] cfgmgr32: Fix CM_Get_Device_ID buffer size handling.

[Erhan Bilgili]

From: Erhan Bilgili <[e-mail]>

CM_Get_Device_ID_ExW() reserved no room for the terminating null in its
size check, which might overwrite outside the buffer bounds.
---
 dlls/cfgmgr32/cfgmgr32.c       |  9 +++++----
 dlls/cfgmgr32/tests/cfgmgr32.c | 21 ++++++++++++++++++++-
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/dlls/cfgmgr32/cfgmgr32.c b/dlls/cfgmgr32/cfgmgr32.c
index c53cf69bfac..ce5747338cf 100644
--- a/dlls/cfgmgr32/cfgmgr32.c
+++ b/dlls/cfgmgr32/cfgmgr32.c
@@ -1868,7 +1868,8 @@ CONFIGRET WINAPI CM_Get_Device_ID_ExW( DEVINST node, WCHAR *buffer, ULONG len, U
     if (*dev.instance) path_len += swprintf( path + path_len, ARRAY_SIZE(path) - path_len, L"\\%s", dev.instance );
 
     if (path_len > len) return CR_BUFFER_SMALL;
-    memcpy( buffer, path, (path_len + 1) * sizeof(WCHAR) );
+    memcpy( buffer, path, path_len * sizeof(WCHAR) );
+    if (path_len < len) buffer[path_len] = 0;
 
     return CR_SUCCESS;
 }
@@ -1881,9 +1882,9 @@ CONFIGRET WINAPI CM_Get_Device_ID_ExA( DEVINST node, char *bufferA, ULONG len, U
     WCHAR *bufferW;
     CONFIGRET ret;
 
-    bufferW = bufferA ? malloc( len * sizeof(WCHAR) ) : NULL;
-    ret = CM_Get_Device_ID_ExW( node, bufferA ? bufferW : NULL, len, flags, machine );
-    if (!ret && bufferA && len && !WideCharToMultiByte( CP_ACP, 0, bufferW, len, bufferA, len, 0, 0 ))
+    bufferW = bufferA ? malloc( (len + 1) * sizeof(WCHAR) ) : NULL;
+    ret = CM_Get_Device_ID_ExW( node, bufferA ? bufferW : NULL, len + 1, flags, machine );
+    if (!ret && bufferA && len && !WideCharToMultiByte( CP_ACP, 0, bufferW, -1, bufferA, len, 0, 0 ))
     {
         if (GetLastError() == ERROR_INSUFFICIENT_BUFFER) ret = CR_BUFFER_SMALL;
         else ret = CR_FAILURE;
diff --git a/dlls/cfgmgr32/tests/cfgmgr32.c b/dlls/cfgmgr32/tests/cfgmgr32.c
index 213eb7f6229..1062874fb73 100644
--- a/dlls/cfgmgr32/tests/cfgmgr32.c
+++ b/dlls/cfgmgr32/tests/cfgmgr32.c
@@ -2905,6 +2905,7 @@ static void test_CM_Open_Device_Interface_Key(void)
 static void test_CM_Locate_DevNode(void)
 {
     WCHAR iface[4096], path[MAX_PATH], instance_id[MAX_PATH];
+    char pathA[MAX_PATH], instance_idA[MAX_PATH];
     DEVINST node, root = 0;
     DWORD size, type, len;
     CONFIGRET ret;
@@ -3000,10 +3001,28 @@ static void test_CM_Locate_DevNode(void)
     todo_wine ok_x4( ret, ==, CR_SUCCESS );
     todo_wine ok_wcs( L"HTREE\\ROOT\\0", path );
 
+    len = wcslen( instance_id );
     memset( path, 0xcd, sizeof(path) );
-    ret = CM_Get_Device_IDW( node, path, ARRAY_SIZE(path), 0 );
+    ret = CM_Get_Device_IDW( node, path, len, 0 );
+    ok_x4( ret, ==, CR_SUCCESS );
+    ok_x4( path[len], ==, 0xcdcd );
+
+    memset( path, 0xcd, sizeof(path) );
+    ret = CM_Get_Device_IDW( node, path, len + 1, 0 );
     ok_x4( ret, ==, CR_SUCCESS );
     ok_wcs( instance_id, path );
+
+    WideCharToMultiByte( CP_ACP, 0, instance_id, -1, instance_idA, ARRAY_SIZE(instance_idA), NULL, NULL );
+
+    len = strlen( instance_idA );
+    memset( path, 0xcd, sizeof(path) );
+    ret = CM_Get_Device_IDA( node, (char *)pathA, len, 0 );
+    ok_x4( ret, ==, CR_BUFFER_SMALL );
+
+    memset( pathA, 0xcd, sizeof(pathA) );
+    ret = CM_Get_Device_IDA( node, (char *)pathA, len + 1, 0 );
+    ok_x4( ret, ==, CR_SUCCESS );
+    ok_str( instance_idA, pathA );
 }
 
 static void test_CM_Open_DevNode_Key(void)
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/11287

[PATCH 0/1] MR11287: cfgmgr32: Fix CM_Get_Device_ID buffer size handling. - approved

[Rémi Bernon (＠rbernon)]

This merge request was approved by Rémi Bernon.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/11287