From 6ddef375c8eb41da3160ac6984f707a19f1da63b Mon Sep 17 00:00:00 2001 From: Tor Lillqvist Date: Thu, 2 Sep 2010 21:56:02 +0300 Subject: [PATCH] Recuce DLL hijack risk on Windows Don't call LoadLibrary() on shell32.dll or kernel32.dll. kernel32.dll is always loaded. Shell32.dll is also already loaded as glib links to functions in it. So just call GetModuleHandle() on them. For mlang.dll in win_iconv.c and winhttp.dll in gwinhttpvfs.c, always try loading them from a complete path, from the Windows system directory. Use the "tool help" API to enumerate modules in gmodule-win32.c. It is present in all Windows versions since Windows 2000, which is all we support anyway. Thus no need to look that API up dynamically. Just link to it normally. We can bin the fallback code that attempts to use the psapi API. --- gio/win32/gwinhttpvfs.c | 15 +++++++-- glib/gutils.c | 6 ++-- glib/win_iconv.c | 14 +++++++-- gmodule/gmodule-win32.c | 82 +++---------------------------------------------- 4 files changed, 33 insertions(+), 84 deletions(-) diff --git a/gio/win32/gwinhttpvfs.c b/gio/win32/gwinhttpvfs.c index 084da2f..a0f747e 100644 --- a/gio/win32/gwinhttpvfs.c +++ b/gio/win32/gwinhttpvfs.c @@ -40,12 +40,23 @@ static GWinHttpDllFuncs funcs; static void lookup_funcs (void) { - HMODULE winhttp; + HMODULE winhttp = NULL; + char winhttp_dll[MAX_PATH + 100]; + int n; if (lookup_done) return; - winhttp = LoadLibrary ("winhttp.dll"); + n = GetSystemDirectory (winhttp_dll, MAX_PATH); + if (n > 0 && n < MAX_PATH) + { + if (winhttp_dll[n-1] != '\\' && + winhttp_dll[n-1] != '/') + strcat (winhttp_dll, "\\"); + strcat (winhttp_dll, "winhttp.dll"); + winhttp = LoadLibrary (winhttp_dll); + } + if (winhttp != NULL) { funcs.pWinHttpCloseHandle = (BOOL (WINAPI *) (HINTERNET)) GetProcAddress (winhttp, "WinHttpCloseHandle"); diff --git a/glib/gutils.c b/glib/gutils.c index 7dc889d..a5794e6 100644 --- a/glib/gutils.c +++ b/glib/gutils.c @@ -2277,13 +2277,15 @@ load_user_special_dirs (void) HANDLE hToken, PWSTR *ppszPath); t_SHGetKnownFolderPath p_SHGetKnownFolderPath; + static const GUID FOLDERID_Downloads = { 0x374de290, 0x123f, 0x4565, { 0x91, 0x64, 0x39, 0xc4, 0x92, 0x5e, 0x46, 0x7b } }; static const GUID FOLDERID_Public = { 0xDFDF76A2, 0xC82A, 0x4D63, { 0x90, 0x6A, 0x56, 0x44, 0xAC, 0x45, 0x73, 0x85 } }; + wchar_t *wcp; - p_SHGetKnownFolderPath = (t_SHGetKnownFolderPath) GetProcAddress (LoadLibrary ("shell32.dll"), + p_SHGetKnownFolderPath = (t_SHGetKnownFolderPath) GetProcAddress (GetModuleHandle ("shell32.dll"), "SHGetKnownFolderPath"); g_user_special_dirs[G_USER_DIRECTORY_DESKTOP] = get_special_folder (CSIDL_DESKTOPDIRECTORY); @@ -2609,7 +2611,7 @@ get_module_for_address (gconstpointer address) if (!beenhere) { p_GetModuleHandleExA = - (t_GetModuleHandleExA) GetProcAddress (LoadLibrary ("kernel32.dll"), + (t_GetModuleHandleExA) GetProcAddress (GetModuleHandle ("kernel32.dll"), "GetModuleHandleExA"); beenhere = TRUE; } diff --git a/glib/win_iconv.c b/glib/win_iconv.c index 416568a..bd7b634 100644 --- a/glib/win_iconv.c +++ b/glib/win_iconv.c @@ -706,10 +706,20 @@ static RFC1766TOLCIDA Rfc1766ToLcidA; static int load_mlang() { - HMODULE h; + HMODULE h = NULL; + char mlang_dll[MAX_PATH + 100]; + int n; if (ConvertINetString != NULL) return TRUE; - h = LoadLibrary("mlang.dll"); + n = GetSystemDirectory(mlang_dll, MAX_PATH); + if (n > 0 && n < MAX_PATH) + { + if (mlang_dll[n-1] != '\\' && + mlang_dll[n-1] != '/') + strcat(mlang_dll, "\\"); + strcat(mlang_dll, "mlang.dll"); + h = LoadLibrary(mlang_dll); + } if (!h) return FALSE; ConvertINetString = (CONVERTINETSTRING)GetProcAddress(h, "ConvertINetString"); diff --git a/gmodule/gmodule-win32.c b/gmodule/gmodule-win32.c index 98d3fb9..439fb5d 100644 --- a/gmodule/gmodule-win32.c +++ b/gmodule/gmodule-win32.c @@ -110,45 +110,22 @@ _g_module_close (gpointer handle, static gpointer find_in_any_module_using_toolhelp (const gchar *symbol_name) { - typedef HANDLE (WINAPI *PFNCREATETOOLHELP32SNAPSHOT)(DWORD, DWORD); - static PFNCREATETOOLHELP32SNAPSHOT pfnCreateToolhelp32Snapshot = NULL; - - typedef BOOL (WINAPI *PFNMODULE32FIRST)(HANDLE, MODULEENTRY32*); - static PFNMODULE32FIRST pfnModule32First= NULL; - - typedef BOOL (WINAPI *PFNMODULE32NEXT)(HANDLE, MODULEENTRY32*); - static PFNMODULE32NEXT pfnModule32Next = NULL; - - static HMODULE kernel32; - HANDLE snapshot; MODULEENTRY32 me32; gpointer p; - if (!pfnCreateToolhelp32Snapshot || !pfnModule32First || !pfnModule32Next) - { - if (!kernel32) - if (!(kernel32 = GetModuleHandle ("kernel32.dll"))) - return NULL; - - if (!(pfnCreateToolhelp32Snapshot = (PFNCREATETOOLHELP32SNAPSHOT) GetProcAddress (kernel32, "CreateToolhelp32Snapshot")) - || !(pfnModule32First = (PFNMODULE32FIRST) GetProcAddress (kernel32, "Module32First")) - || !(pfnModule32Next = (PFNMODULE32NEXT) GetProcAddress (kernel32, "Module32Next"))) - return NULL; - } - - if ((snapshot = (*pfnCreateToolhelp32Snapshot) (TH32CS_SNAPMODULE, 0)) == (HANDLE) -1) + if ((snapshot = CreateToolhelp32Snapshot (TH32CS_SNAPMODULE, 0)) == (HANDLE) -1) return NULL; me32.dwSize = sizeof (me32); p = NULL; - if ((*pfnModule32First) (snapshot, &me32)) + if (Module32First (snapshot, &me32)) { do { if ((p = GetProcAddress (me32.hModule, symbol_name)) != NULL) break; - } while ((*pfnModule32Next) (snapshot, &me32)); + } while (Module32Next (snapshot, &me32)); } CloseHandle (snapshot); @@ -157,62 +134,11 @@ find_in_any_module_using_toolhelp (const gchar *symbol_name) } static gpointer -find_in_any_module_using_psapi (const gchar *symbol_name) -{ - static HMODULE psapi = NULL; - - typedef BOOL (WINAPI *PFNENUMPROCESSMODULES) (HANDLE, HMODULE *, DWORD, LPDWORD) ; - static PFNENUMPROCESSMODULES pfnEnumProcessModules = NULL; - - HMODULE *modules; - HMODULE dummy; - gint i, size; - DWORD needed; - - gpointer p; - - if (!pfnEnumProcessModules) - { - if (!psapi) - if ((psapi = LoadLibrary ("psapi.dll")) == NULL) - return NULL; - - if (!(pfnEnumProcessModules = (PFNENUMPROCESSMODULES) GetProcAddress (psapi, "EnumProcessModules"))) - return NULL; - } - - if (!(*pfnEnumProcessModules) (GetCurrentProcess (), &dummy, - sizeof (HMODULE), &needed)) - return NULL; - - size = needed + 10 * sizeof (HMODULE); - modules = g_malloc (size); - - if (!(*pfnEnumProcessModules) (GetCurrentProcess (), modules, - size, &needed) - || needed > size) - { - g_free (modules); - return NULL; - } - - p = NULL; - for (i = 0; i < needed / sizeof (HMODULE); i++) - if ((p = GetProcAddress (modules[i], symbol_name)) != NULL) - break; - - g_free (modules); - - return p; -} - -static gpointer find_in_any_module (const gchar *symbol_name) { gpointer result; - if ((result = find_in_any_module_using_toolhelp (symbol_name)) == NULL - && (result = find_in_any_module_using_psapi (symbol_name)) == NULL) + if ((result = find_in_any_module_using_toolhelp (symbol_name)) == NULL) return NULL; else return result; -- 2.7.4