Alexandre Julliard : kernel32: Fix buffer overflows in K32GetModuleFileNameExA/W.

[Alexandre Julliard]

Module: wine
Branch: master
Commit: d08f34cd8ecd883a0f0c6bd9b150d92407f0f7c9
URL:    http://source.winehq.org/git/wine.git/?a=commit;h=d08f34cd8ecd883a0f0c6bd9b1...

Author: Alexandre Julliard <julliard(a)winehq.org>
Date:   Mon Apr 30 14:19:57 2012 +0200

kernel32: Fix buffer overflows in K32GetModuleFileNameExA/W.

---

 dlls/kernel32/module.c        |   31 ++++++++++++++++++++++++-----
 dlls/psapi/tests/psapi_main.c |   42 +++++++++++++++++++++++++++++++++++++---
 2 files changed, 63 insertions(+), 10 deletions(-)

diff --git a/dlls/kernel32/module.c b/dlls/kernel32/module.c
index 6b91a4c..57a24ba 100644
--- a/dlls/kernel32/module.c
+++ b/dlls/kernel32/module.c
@@ -1247,17 +1247,26 @@ DWORD WINAPI K32GetModuleFileNameExW(HANDLE process, HMODULE module,
                                      LPWSTR file_name, DWORD size)
 {
     LDR_MODULE ldr_module;
+    DWORD len;
+
+    if (!size) return 0;
 
     if(!get_ldr_module(process, module, &ldr_module))
         return 0;
 
-    size = min(ldr_module.FullDllName.Length / sizeof(WCHAR), size);
+    len = ldr_module.FullDllName.Length / sizeof(WCHAR);
+    if (size <= len)
+    {
+        len = size;
+        size--;
+    }
+
     if (!ReadProcessMemory(process, ldr_module.FullDllName.Buffer,
                            file_name, size * sizeof(WCHAR), NULL))
         return 0;
 
     file_name[size] = 0;
-    return size;
+    return len;
 }
 
 /***********************************************************************
@@ -1267,32 +1276,42 @@ DWORD WINAPI K32GetModuleFileNameExA(HANDLE process, HMODULE module,
                                      LPSTR file_name, DWORD size)
 {
     WCHAR *ptr;
+    DWORD len;
 
     TRACE("(hProcess=%p, hModule=%p, %p, %d)\n", process, module, file_name, size);
 
-    if (!file_name || !size) return 0;
+    if (!file_name || !size)
+    {
+        SetLastError( ERROR_INVALID_PARAMETER );
+        return 0;
+    }
 
     if ( process == GetCurrentProcess() )
     {
-        DWORD len = GetModuleFileNameA( module, file_name, size );
+        len = GetModuleFileNameA( module, file_name, size );
         if (size) file_name[size - 1] = '\0';
         return len;
     }
 
     if (!(ptr = HeapAlloc(GetProcessHeap(), 0, size * sizeof(WCHAR)))) return 0;
 
-    if (!K32GetModuleFileNameExW(process, module, ptr, size))
+    len = K32GetModuleFileNameExW(process, module, ptr, size);
+    if (!len)
     {
         file_name[0] = '\0';
     }
     else
     {
         if (!WideCharToMultiByte( CP_ACP, 0, ptr, -1, file_name, size, NULL, NULL ))
+        {
             file_name[size - 1] = 0;
+            len = size;
+        }
+        else if (len < size) len = strlen( file_name );
     }
 
     HeapFree(GetProcessHeap(), 0, ptr);
-    return strlen(file_name);
+    return len;
 }
 
 /***********************************************************************
diff --git a/dlls/psapi/tests/psapi_main.c b/dlls/psapi/tests/psapi_main.c
index 1944065..8db695e 100644
--- a/dlls/psapi/tests/psapi_main.c
+++ b/dlls/psapi/tests/psapi_main.c
@@ -49,6 +49,7 @@ static BOOL  (WINAPI *pEnumProcesses)(DWORD*, DWORD, DWORD*);
 static BOOL  (WINAPI *pEnumProcessModules)(HANDLE, HMODULE*, DWORD, LPDWORD);
 static DWORD (WINAPI *pGetModuleBaseNameA)(HANDLE, HMODULE, LPSTR, DWORD);
 static DWORD (WINAPI *pGetModuleFileNameExA)(HANDLE, HMODULE, LPSTR, DWORD);
+static DWORD (WINAPI *pGetModuleFileNameExW)(HANDLE, HMODULE, LPWSTR, DWORD);
 static BOOL  (WINAPI *pGetModuleInformation)(HANDLE, HMODULE, LPMODULEINFO, DWORD);
 static DWORD (WINAPI *pGetMappedFileNameA)(HANDLE, LPVOID, LPSTR, DWORD);
 static DWORD (WINAPI *pGetMappedFileNameW)(HANDLE, LPVOID, LPWSTR, DWORD);
@@ -67,6 +68,7 @@ static BOOL InitFunctionPtrs(HMODULE hpsapi)
     PSAPI_GET_PROC(EnumProcesses);
     PSAPI_GET_PROC(GetModuleBaseNameA);
     PSAPI_GET_PROC(GetModuleFileNameExA);
+    PSAPI_GET_PROC(GetModuleFileNameExW);
     PSAPI_GET_PROC(GetModuleInformation);
     PSAPI_GET_PROC(GetMappedFileNameA);
     PSAPI_GET_PROC(GetMappedFileNameW);
@@ -471,18 +473,22 @@ static void test_GetModuleFileNameEx(void)
 {
     HMODULE hMod = GetModuleHandle(NULL);
     char szModExPath[MAX_PATH+1], szModPath[MAX_PATH+1];
+    WCHAR buffer[MAX_PATH];
     DWORD ret;
-    
+
     SetLastError(0xdeadbeef);
-    pGetModuleFileNameExA(NULL, hMod, szModExPath, sizeof(szModExPath));
+    ret = pGetModuleFileNameExA(NULL, hMod, szModExPath, sizeof(szModExPath));
+    ok( !ret, "GetModuleFileNameExA succeeded\n" );
     ok(GetLastError() == ERROR_INVALID_HANDLE, "expected error=ERROR_INVALID_HANDLE but got %d\n", GetLastError());
 
     SetLastError(0xdeadbeef);
-    pGetModuleFileNameExA(hpQI, hMod, szModExPath, sizeof(szModExPath));
+    ret = pGetModuleFileNameExA(hpQI, hMod, szModExPath, sizeof(szModExPath));
+    ok( !ret, "GetModuleFileNameExA succeeded\n" );
     ok(GetLastError() == ERROR_ACCESS_DENIED, "expected error=ERROR_ACCESS_DENIED but got %d\n", GetLastError());
 
     SetLastError(0xdeadbeef);
-    pGetModuleFileNameExA(hpQV, hBad, szModExPath, sizeof(szModExPath));
+    ret = pGetModuleFileNameExA(hpQV, hBad, szModExPath, sizeof(szModExPath));
+    ok( !ret, "GetModuleFileNameExA succeeded\n" );
     ok(GetLastError() == ERROR_INVALID_HANDLE, "expected error=ERROR_INVALID_HANDLE but got %d\n", GetLastError());
 
     ret = pGetModuleFileNameExA(hpQV, NULL, szModExPath, sizeof(szModExPath));
@@ -492,6 +498,34 @@ static void test_GetModuleFileNameEx(void)
     GetModuleFileNameA(NULL, szModPath, sizeof(szModPath));
     ok(!strncmp(szModExPath, szModPath, MAX_PATH), 
        "szModExPath=\"%s\" szModPath=\"%s\"\n", szModExPath, szModPath);
+
+    SetLastError(0xdeadbeef);
+    memset( szModExPath, 0xcc, sizeof(szModExPath) );
+    ret = pGetModuleFileNameExA(hpQV, NULL, szModExPath, 4 );
+    ok( ret == 4, "wrong length %u\n", ret );
+    ok( broken(szModExPath[3]) /*w2kpro*/ || strlen(szModExPath) == 3,
+        "szModExPath=\"%s\" ret=%d\n", szModExPath, ret );
+    ok(GetLastError() == 0xdeadbeef, "got error %d\n", GetLastError());
+
+    SetLastError(0xdeadbeef);
+    ret = pGetModuleFileNameExA(hpQV, NULL, szModExPath, 0 );
+    ok( ret == 0, "wrong length %u\n", ret );
+    ok(GetLastError() == ERROR_INVALID_PARAMETER, "got error %d\n", GetLastError());
+
+    SetLastError(0xdeadbeef);
+    memset( buffer, 0xcc, sizeof(buffer) );
+    ret = pGetModuleFileNameExW(hpQV, NULL, buffer, 4 );
+    ok( ret == 4, "wrong length %u\n", ret );
+    ok( broken(buffer[3]) /*w2kpro*/ || lstrlenW(buffer) == 3,
+        "buffer=%s ret=%d\n", wine_dbgstr_w(buffer), ret );
+    ok(GetLastError() == 0xdeadbeef, "got error %d\n", GetLastError());
+
+    SetLastError(0xdeadbeef);
+    buffer[0] = 0xcc;
+    ret = pGetModuleFileNameExW(hpQV, NULL, buffer, 0 );
+    ok( ret == 0, "wrong length %u\n", ret );
+    ok(GetLastError() == 0xdeadbeef, "got error %d\n", GetLastError());
+    ok( buffer[0] == 0xcc, "buffer modified %s\n", wine_dbgstr_w(buffer) );
 }
 
 static void test_GetModuleBaseName(void)