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.

diff --git a/gio/win32/gwinhttpvfs.c b/gio/win32/gwinhttpvfs.c
index 084da2f058a987cba833d95678ca5987cd504d74..a0f747e857d0babf9b3c1bb660f5710bd28d3390 100644 (file)
--- 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 7dc889d75ac28d09d4ea29591a898d6ecac3ae9e..a5794e681f94df2546cf8a2a0ebcf7937052c309 100644 (file)
--- 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 416568aa04dbcd78b82b4cffffd7b81a3dcbe7d4..bd7b634b5d3410d0920ffe8546f0e9945392ba2f 100644 (file)
--- a/glib/win_iconv.c
+++ b/glib/win_iconv.c
@@ -706,10 +706,20 @@ static RFC1766TOLCIDA Rfc1766ToLcidA;
 static int\r
 load_mlang()\r
 {\r
-    HMODULE h;\r
+    HMODULE h = NULL;\r
+    char mlang_dll[MAX_PATH + 100];\r
+    int n;\r
     if (ConvertINetString != NULL)\r
         return TRUE;\r
-    h = LoadLibrary("mlang.dll");\r
+    n = GetSystemDirectory(mlang_dll, MAX_PATH);\r
+    if (n > 0 && n < MAX_PATH)\r
+    {\r
+        if (mlang_dll[n-1] != '\\' &&\r
+            mlang_dll[n-1] != '/')\r
+            strcat(mlang_dll, "\\");\r
+        strcat(mlang_dll, "mlang.dll");\r
+        h = LoadLibrary(mlang_dll);\r
+    }\r
     if (!h)\r
         return FALSE;\r
     ConvertINetString = (CONVERTINETSTRING)GetProcAddress(h, "ConvertINetString");\r

diff --git a/gmodule/gmodule-win32.c b/gmodule/gmodule-win32.c
index 98d3fb9c600b1b131a8f257bd086d0a75a2988ba..439fb5d0f9347f08a268f9daae50503b84732b4a 100644 (file)
--- 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);
@@ -156,63 +133,12 @@ find_in_any_module_using_toolhelp (const gchar *symbol_name)
   return p;
 }
 
-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;