[PATCH v2 0/2] MR8041: kernelbase: Fixed stack overflow of DefineDosDeviceW()

[Wei Xie (＠wayne)]

1. Changed fixed-size buffer to dynamic allocation for link_name to
prevent buffer overflow
2. Added proper memory cleanup with HeapFree in all error paths
3. Fixed potential null pointer dereference when device is NULL
4. Improved error handling by checking memory allocation success

The original code used a fixed-size buffer (15 chars) which could
overflow with long device names. The new version dynamically allocates
memory based on actual device name length, includes proper cleanup, and 
handles edge cases better.

Signed-off-by: Wei Xie <xiewei(a)uniontech.com>

--
  v2: fix: add missing DefineDosDeviceW test cases
      kernelbase: fix memory allocation in DefineDosDeviceW
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/8041

[PATCH v2 1/2] kernelbase: fix memory allocation in DefineDosDeviceW

[Wei Xie]

From: Wei Xie <xiewei(a)uniontech.com>

1. Changed fixed-size buffer to dynamic allocation for link_name to
prevent buffer overflow
2. Added proper memory cleanup with HeapFree in all error paths
3. Fixed potential null pointer dereference when device is NULL
4. Improved error handling by checking memory allocation success

The original code used a fixed-size buffer (15 chars) which could
overflow with long device names. The new version dynamically allocates
memory based on actual device name length, includes proper cleanup, and
handles edge cases better.

Signed-off-by: Wei Xie <xiewei(a)uniontech.com>
---
 dlls/kernelbase/volume.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/dlls/kernelbase/volume.c b/dlls/kernelbase/volume.c
index cbe4dc05db4..fa9830c4308 100644
--- a/dlls/kernelbase/volume.c
+++ b/dlls/kernelbase/volume.c
@@ -365,7 +365,7 @@ err_ret:
  */
 BOOL WINAPI DECLSPEC_HOTPATCH DefineDosDeviceW( DWORD flags, const WCHAR *device, const WCHAR *target )
 {
-    WCHAR link_name[15] = L"\\DosDevices\\";
+    WCHAR *link_name = NULL;
     UNICODE_STRING nt_name, nt_target;
     OBJECT_ATTRIBUTES attr;
     NTSTATUS status;
@@ -376,17 +376,28 @@ BOOL WINAPI DECLSPEC_HOTPATCH DefineDosDeviceW( DWORD flags, const WCHAR *device
     if (flags & ~(DDD_RAW_TARGET_PATH | DDD_REMOVE_DEFINITION))
         FIXME("Ignoring flags %#lx.\n", flags & ~(DDD_RAW_TARGET_PATH | DDD_REMOVE_DEFINITION));
 
-    lstrcatW( link_name, device );
+    if (!(link_name = HeapAlloc( GetProcessHeap(), 0, sizeof(L"\\DosDevices\\") + (device ? lstrlenW(device)*sizeof(WCHAR) : 0))))
+    {
+        SetLastError(ERROR_OUTOFMEMORY);
+        return FALSE;
+    }
+
+    lstrcpyW( link_name, L"\\DosDevices\\" );
+    if (device) lstrcatW( link_name, device );
     RtlInitUnicodeString( &nt_name, link_name );
     InitializeObjectAttributes( &attr, &nt_name, OBJ_CASE_INSENSITIVE | OBJ_PERMANENT, 0, NULL );
     if (flags & DDD_REMOVE_DEFINITION)
     {
         if (!set_ntstatus( NtOpenSymbolicLinkObject( &handle, DELETE, &attr ) ))
+        {
+            HeapFree( GetProcessHeap(), 0, link_name );
             return FALSE;
+        }
 
         status = NtMakeTemporaryObject( handle );
         NtClose( handle );
 
+        HeapFree( GetProcessHeap(), 0, link_name );
         return set_ntstatus( status );
     }
 
@@ -395,6 +406,7 @@ BOOL WINAPI DECLSPEC_HOTPATCH DefineDosDeviceW( DWORD flags, const WCHAR *device
         if (!RtlDosPathNameToNtPathName_U( target, &nt_target, NULL, NULL))
         {
             SetLastError( ERROR_PATH_NOT_FOUND );
+            HeapFree( GetProcessHeap(), 0, link_name );
             return FALSE;
         }
     }
@@ -403,6 +415,8 @@ BOOL WINAPI DECLSPEC_HOTPATCH DefineDosDeviceW( DWORD flags, const WCHAR *device
 
     if (!(status = NtCreateSymbolicLinkObject( &handle, SYMBOLIC_LINK_ALL_ACCESS, &attr, &nt_target )))
         NtClose( handle );
+
+    HeapFree( GetProcessHeap(), 0, link_name );
     return set_ntstatus( status );
 }
 
-- 
GitLab


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

[PATCH v2 2/2] fix: add missing DefineDosDeviceW test cases

[Wei Xie]

From: Wei Xie <xiewei(a)uniontech.com>

Added test cases for DefineDosDeviceW function to verify proper handling
of raw target paths and definition removal. The new tests check:
1. Creating a DOS device with DDD_RAW_TARGET_PATH flag
2. Proper removal of the DOS device definition
These tests ensure correct behavior of device path mapping functionality
in Windows compatibility layer.

Signed-off-by: Wei Xie <xiewei(a)uniontech.com>
---
 dlls/kernel32/tests/volume.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/dlls/kernel32/tests/volume.c b/dlls/kernel32/tests/volume.c
index e331b373206..70a810f8f16 100644
--- a/dlls/kernel32/tests/volume.c
+++ b/dlls/kernel32/tests/volume.c
@@ -188,6 +188,12 @@ static void test_dos_devices(void)
     ret = QueryDosDeviceA( drivestr, buf, sizeof(buf) );
     ok(!ret, "expected failure\n");
     ok(GetLastError() == ERROR_FILE_NOT_FOUND, "got error %lu\n", GetLastError());
+
+    ret = DefineDosDeviceW ( DDD_RAW_TARGET_PATH, L"C:/windows/", L"\\Device\\C:/windows/" );
+    ok(ret, "got error %lu\n", GetLastError());
+
+    ret = DefineDosDeviceW(DDD_REMOVE_DEFINITION, L"C:/windows/", NULL);
+    ok(ret, "failed to remove C:/windows/, error %lu\n", GetLastError());
 }
 
 static void test_FindFirstVolume(void)
-- 
GitLab

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