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

[Wei Xie (＠wayne)]

Signed-off-by: Wei Xie

[PATCH 1/2] kernelbase: Fixed stack overflow of DefineDosDeviceW()

[Wei Xie]

From: Wei Xie

Signed-off-by: Wei Xie 
---
 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 );
 }

[PATCH 2/2] kernel32/tests: Add some tests for DefineDosDeviceW()

[Wei Xie]

From: Wei Xie

Signed-off-by: Wei Xie 
---
 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)

[Jinoh Kang (＠iamahuman)]

Interesting, which app requires this? (Otherwise we can just reject long device name with a FIXME).

[Wei Xie (＠wayne)]

This bug is reproduced on QQ, and the probability of recurrence is relatively low. It is not clear what is the specific reason for this code branch.