Win32: change DLL search path to accomodate extcap and Npcap

Extcap executables require libwsutil.dll from the program directory.
These were loaded by setting the PATH environment variable, but this
is not thread-safe (and caused sporadic tests failures as a result).
Use SetDllDirectory instead, this also prevents loading DLL files
from arbitrary directories in PATH.

To make this work, the search logic for Npcap has to be modified to
avoid relying on SetDllDirectory. This implies that Npcap cannot be
used on Windows 7 anymore until KB2533623 (July 2011) is applied.

Change-Id: I3fc42ff76e75ae162b6dd31103451fb8f71c09e6
Reviewed-on: https://code.wireshark.org/review/30804
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>

2 files changed, 34 insertions, 34 deletions

diff --git a/wsutil/file_util.c b/wsutil/file_util.c
index 4045b978a8..b04568d0e3 100644
--- a/wsutil/file_util.c
+++ b/wsutil/file_util.c
@@ -493,22 +493,14 @@ ws_init_dll_search_path()
 {
     gboolean dll_dir_set = FALSE;
     wchar_t *program_path_w;
-    wchar_t npcap_path_w[MAX_PATH];
-    unsigned int retval;
-
-    dll_dir_set = SetDllDirectory(_T(""));
-    if (dll_dir_set) {
-        /* Add Npcap folder to libraries search path. */
-        retval = GetSystemDirectoryW(npcap_path_w, MAX_PATH);
-        if (0 < retval && retval <= MAX_PATH) {
-            wcscat_s(npcap_path_w, MAX_PATH, L"\\Npcap");
-            dll_dir_set = SetDllDirectory(npcap_path_w);
-        }
-    }
 
-    if (!dll_dir_set && init_dll_load_paths()) {
+    /* Remove the current directory from the default DLL search path. */
+    SetDllDirectory(_T(""));
+
+    if (init_dll_load_paths()) {
+        /* Ensure that extcap executables can find wsutil, etc. */
         program_path_w = g_utf8_to_utf16(program_path, -1, NULL, NULL, NULL);
-        SetCurrentDirectory(program_path_w);
+        dll_dir_set = SetDllDirectory(program_path_w);
         g_free(program_path_w);
     }
 
@@ -561,6 +553,30 @@ ws_load_library(const gchar *library_name)
     return NULL;
 }
 
+static GModule *
+load_npcap_module(const gchar *full_path, GModuleFlags flags)
+{
+    /*
+     * Npcap's wpcap.dll requires packet.dll from the same directory. Either
+     * SetDllDirectory or SetCurrentDirectory could make this work, but it
+     * interferes with other uses of these settings. LoadLibraryEx is ideal as
+     * it can be configured to put the directory containing the DLL to the
+     * search path. Unfortunately g_module_open uses LoadLibrary internally, so
+     * as a workaround manually load the Npcap libraries first and then use
+     * g_module_open to obtain a GModule for the loaded library.
+     */
+
+    wchar_t *wpath = g_utf8_to_utf16(full_path, -1, NULL, NULL, NULL);
+    HMODULE module = LoadLibraryEx(wpath, NULL, LOAD_LIBRARY_SEARCH_DEFAULT_DIRS | LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR);
+    g_free(wpath);
+    if (!module) {
+        return NULL;
+    }
+    GModule *mod = g_module_open(full_path, flags);
+    FreeLibrary(module);
+    return mod;
+}
+
 GModule *
 ws_module_open(gchar *module_name, GModuleFlags flags)
 {
@@ -575,8 +591,8 @@ ws_module_open(gchar *module_name, GModuleFlags flags)
 
     if (full_path) {
         mod = g_module_open(full_path, flags);
+        g_free(full_path);
         if (mod) {
-            g_free(full_path);
             return mod;
         }
     }
@@ -585,9 +601,9 @@ ws_module_open(gchar *module_name, GModuleFlags flags)
     full_path = g_module_build_path(npcap_path, module_name);
 
     if (full_path) {
-        mod = g_module_open(full_path, flags);
+        mod = load_npcap_module(full_path, flags);
+        g_free(full_path);
         if (mod) {
-            g_free(full_path);
             return mod;
         }
     }
@@ -597,8 +613,8 @@ ws_module_open(gchar *module_name, GModuleFlags flags)
 
     if (full_path) {
         mod = g_module_open(full_path, flags);
+        g_free(full_path);
         if (mod) {
-            g_free(full_path);
             return mod;
         }
     }
diff --git a/wsutil/ws_pipe.c b/wsutil/ws_pipe.c
index 35cc7a242c..6f19439ab0 100644
--- a/wsutil/ws_pipe.c
+++ b/wsutil/ws_pipe.c
@@ -137,9 +137,6 @@ gboolean ws_pipe_spawn_sync(const gchar *working_directory, const gchar *command
     HANDLE child_stdout_wr = NULL;
     HANDLE child_stderr_rd = NULL;
     HANDLE child_stderr_wr = NULL;
-
-    const gchar *oldpath = g_getenv("PATH");
-    gchar *newpath = NULL;
 #else
     gint exit_status = 0;
 #endif
@@ -148,9 +145,6 @@ gboolean ws_pipe_spawn_sync(const gchar *working_directory, const gchar *command
     GString *spawn_string = g_string_new("");
 
 #ifdef _WIN32
-    newpath = g_strdup_printf("%s;%s", g_strescape(get_progfile_dir(), NULL), oldpath);
-    g_setenv("PATH", newpath, TRUE);
-
     argv[0] = g_strescape(command, NULL);
 #else
     argv[0] = g_strdup(command);
@@ -290,8 +284,6 @@ gboolean ws_pipe_spawn_sync(const gchar *working_directory, const gchar *command
     }
     else
         status = FALSE;
-
-    g_setenv("PATH", oldpath, TRUE);
 #else
 
     GSpawnFlags flags = (GSpawnFlags)0;
@@ -354,12 +346,6 @@ GPid ws_pipe_spawn_async(ws_pipe_t *ws_pipe, GPtrArray *args)
     HANDLE child_stderr_rd = NULL;
     HANDLE child_stderr_wr = NULL;
 
-    const gchar *oldpath = g_getenv("PATH");
-    gchar *newpath = NULL;
-
-    newpath = g_strdup_printf("%s;%s", g_strescape(get_progfile_dir(), NULL), oldpath);
-    g_setenv("PATH", newpath, TRUE);
-
     sa.nLength = sizeof(SECURITY_ATTRIBUTES);
     sa.bInheritHandle = TRUE;
     sa.lpSecurityDescriptor = NULL;
@@ -415,8 +401,6 @@ GPid ws_pipe_spawn_async(ws_pipe_t *ws_pipe, GPtrArray *args)
         ws_pipe->threadId = processInfo.hThread;
         pid = processInfo.hProcess;
     }
-
-    g_setenv("PATH", oldpath, TRUE);
 #else
 
     spawn_args = g_string_sized_new(200);