From d08f34cd8ecd883a0f0c6bd9b150d92407f0f7c9 Mon Sep 17 00:00:00 2001 From: Alexandre Julliard Date: Mon, 30 Apr 2012 14:19:57 +0200 Subject: [PATCH] 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 6b91a4cda1d..57a24ba1514 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 19440652ff3..8db695edb2d 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)