win32-utils: Explicitly list inherited handles

Windows processes inherit all inheritable handles when a new process is
created using CreateProcess() with bInheritHandles set to TRUE. This can
lead to undesired object lifetime extension. That is, the child process
will keep ineritable handles alive even if it does not use them. Up to
Windows Vista it was not possible explicitly list handles that should be
inherited. Wireshark no longer works on Windows releases earlier than
Vista, so use the new API without checking Windows version.

Require all callers to win32_create_process() to pass in the list of
handles to inherit. Set the listed handles as inheritable shortly before
calling CreateProcess() and set them as not inheritable shortly after
the process is created. This minimizes possibility for other callers
(especially in 3rd party libraries) to inherit handles by accident.

Do not terminate mmdbresolve process on exit. Instead rely on process
exit when EOF is received on standard input. Previously the EOF was
never received because mmdbresolve inherited both ends of standard input
pipe, i.e. the fact that Wireshark closed the write end was not observed
by mmdbresolve because mmdbresolve kept write handle the standard input
pipe open.

3 files changed, 71 insertions, 9 deletions

diff --git a/wsutil/win32-utils.c b/wsutil/win32-utils.c
index fe8dcfd380..fe49001c4f 100644
--- a/wsutil/win32-utils.c
+++ b/wsutil/win32-utils.c
@@ -219,10 +219,13 @@ static void win32_kill_child_on_exit(HANDLE child_handle) {
     }
 }
 
-BOOL win32_create_process(const char *application_name, const char *command_line, LPSECURITY_ATTRIBUTES process_attributes, LPSECURITY_ATTRIBUTES thread_attributes, BOOL inherit_handles, DWORD creation_flags, LPVOID environment, const char *current_directory, LPSTARTUPINFO startup_info, LPPROCESS_INFORMATION process_information)
+BOOL win32_create_process(const char *application_name, const char *command_line, LPSECURITY_ATTRIBUTES process_attributes, LPSECURITY_ATTRIBUTES thread_attributes, size_t n_inherit_handles, HANDLE *inherit_handles, DWORD creation_flags, LPVOID environment, const char *current_directory, LPSTARTUPINFO startup_info, LPPROCESS_INFORMATION process_information)
 {
     gunichar2 *wappname = NULL, *wcurrentdirectory = NULL;
     gunichar2 *wcommandline = g_utf8_to_utf16(command_line, -1, NULL, NULL, NULL);
+    LPPROC_THREAD_ATTRIBUTE_LIST attribute_list = NULL;
+    STARTUPINFOEX startup_infoex;
+    size_t i;
     // CREATE_SUSPENDED: Suspend the child so that we can cleanly call
     //     AssignProcessToJobObject.
     DWORD wcreationflags = creation_flags|CREATE_SUSPENDED;
@@ -243,15 +246,54 @@ BOOL win32_create_process(const char *application_name, const char *command_line
     if (current_directory) {
         wcurrentdirectory = g_utf8_to_utf16(current_directory, -1, NULL, NULL, NULL);
     }
+    if (n_inherit_handles > 0) {
+        size_t attr_size = 0;
+        BOOL success;
+        success = InitializeProcThreadAttributeList(NULL, 1, 0, &attr_size);
+        if (success || (GetLastError() == ERROR_INSUFFICIENT_BUFFER)) {
+            attribute_list = g_malloc(attr_size);
+            success = InitializeProcThreadAttributeList(attribute_list, 1, 0, &attr_size);
+        }
+        if (success && (attribute_list != NULL)) {
+            success = UpdateProcThreadAttribute(attribute_list, 0, PROC_THREAD_ATTRIBUTE_HANDLE_LIST,
+                inherit_handles, n_inherit_handles * sizeof(HANDLE), NULL, NULL);
+        }
+        if (!success && (attribute_list != NULL)) {
+            DeleteProcThreadAttributeList(attribute_list);
+            g_free(attribute_list);
+            attribute_list = NULL;
+        }
+    }
+    memset(&startup_infoex, 0, sizeof(startup_infoex));
+    startup_infoex.StartupInfo = *startup_info;
+    startup_infoex.StartupInfo.cb = sizeof(startup_infoex);
+    startup_infoex.lpAttributeList = attribute_list;
+    wcreationflags |= EXTENDED_STARTUPINFO_PRESENT;
+    for (i = 0; i < n_inherit_handles; i++) {
+        SetHandleInformation(inherit_handles[i], HANDLE_FLAG_INHERIT, HANDLE_FLAG_INHERIT);
+    }
     BOOL cp_res = CreateProcess(wappname, wcommandline, process_attributes, thread_attributes,
-        inherit_handles, wcreationflags, environment, wcurrentdirectory, startup_info,
-        process_information);
+        (n_inherit_handles > 0) ? TRUE : FALSE, wcreationflags, environment, wcurrentdirectory,
+        &startup_infoex.StartupInfo, process_information);
+    /* While this function makes the created process inherit only the explicitly
+     * listed handles, there can be other functions (in 3rd party libraries)
+     * that create processes inheriting all inheritable handles. To minimize
+     * number of unwanted handle duplicates (handle duplicate can extend object
+     * lifetime, e.g. pipe write end) created that way clear the inherit flag.
+     */
+    for (i = 0; i < n_inherit_handles; i++) {
+        SetHandleInformation(inherit_handles[i], HANDLE_FLAG_INHERIT, 0);
+    }
     if (cp_res) {
         win32_kill_child_on_exit(process_information->hProcess);
         ResumeThread(process_information->hThread);
     }
     // XXX Else try again if CREATE_BREAKAWAY_FROM_JOB and GetLastError() == ERROR_ACCESS_DENIED?
 
+    if (attribute_list) {
+        DeleteProcThreadAttributeList(attribute_list);
+        g_free(attribute_list);
+    }
     g_free(wappname);
     g_free(wcommandline);
     g_free(wcurrentdirectory);
diff --git a/wsutil/win32-utils.h b/wsutil/win32-utils.h
index 2ad6411fda..3cf8d3ee43 100644
--- a/wsutil/win32-utils.h
+++ b/wsutil/win32-utils.h
@@ -60,11 +60,19 @@ const char * win32strexception(DWORD exception);
 /**
  * @brief ws_pipe_create_process Create a process and assign it to the main application
  *        job object so that it will be killed when the main application exits.
+ *
+ * In order to limit unwanted handle duplicates in subprocesses all handles should be
+ * created as not inheritable and passed in the inherit_handles array. This function
+ * marks the handles as inheritable for as short time as possible. Note that handles
+ * passed to this function will have the inheritable flag cleared on exit. Processes
+ * created with this function inherit only the provided handles.
+ *
  * @param application_name Application name. Will be converted to its UTF-16 equivalent or NULL.
  * @param command_line Command line. Will be converted to its UTF-16 equivalent.
  * @param process_attributes Same as CreateProcess.
  * @param thread_attributes Same as CreateProcess.
- * @param inherit_handles Same as CreateProcess.
+ * @param n_inherit_handles Number of handles the child process will inherit.
+ * @param inherit_handles Handles the child process will inherit.
  * @param creation_flags Will be ORed with CREATE_SUSPENDED|CREATE_BREAKAWAY_FROM_JOB.
  * @param environment Same as CreateProcess.
  * @param current_directory Current directory. Will be converted to its UTF-16 equivalent or NULL.
@@ -75,7 +83,8 @@ const char * win32strexception(DWORD exception);
 WS_DLL_PUBLIC
 BOOL win32_create_process(const char *application_name, const char *command_line,
     LPSECURITY_ATTRIBUTES process_attributes, LPSECURITY_ATTRIBUTES thread_attributes,
-    BOOL inherit_handles, DWORD creation_flags, LPVOID environment,
+    size_t n_inherit_handles, HANDLE *inherit_handles,
+    DWORD creation_flags, LPVOID environment,
     const char *current_directory, LPSTARTUPINFO startup_info, LPPROCESS_INFORMATION process_information
 );
 
diff --git a/wsutil/ws_pipe.c b/wsutil/ws_pipe.c
index 74c39103d0..da3b3a274b 100644
--- a/wsutil/ws_pipe.c
+++ b/wsutil/ws_pipe.c
@@ -243,6 +243,7 @@ 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;
+    HANDLE inherit_handles[2];
 
     OVERLAPPED stdout_overlapped;
     OVERLAPPED stderr_overlapped;
@@ -281,7 +282,7 @@ gboolean ws_pipe_spawn_sync(const gchar *working_directory, const gchar *command
 
     memset(&sa, 0, sizeof(SECURITY_ATTRIBUTES));
     sa.nLength = sizeof(SECURITY_ATTRIBUTES);
-    sa.bInheritHandle = TRUE;
+    sa.bInheritHandle = FALSE;
     sa.lpSecurityDescriptor = NULL;
 
     if (!ws_pipe_create_overlapped_read(&child_stdout_rd, &child_stdout_wr, &sa, 0))
@@ -306,6 +307,9 @@ gboolean ws_pipe_spawn_sync(const gchar *working_directory, const gchar *command
         return FALSE;
     }
 
+    inherit_handles[0] = child_stderr_wr;
+    inherit_handles[1] = child_stdout_wr;
+
     memset(&processInfo, 0, sizeof(PROCESS_INFORMATION));
     memset(&info, 0, sizeof(STARTUPINFO));
 
@@ -315,7 +319,8 @@ gboolean ws_pipe_spawn_sync(const gchar *working_directory, const gchar *command
     info.dwFlags = STARTF_USESTDHANDLES | STARTF_USESHOWWINDOW;
     info.wShowWindow = SW_HIDE;
 
-    if (win32_create_process(NULL, command_line, NULL, NULL, TRUE, CREATE_NEW_CONSOLE, NULL, working_directory, &info, &processInfo))
+    if (win32_create_process(NULL, command_line, NULL, NULL, G_N_ELEMENTS(inherit_handles), inherit_handles,
+                             CREATE_NEW_CONSOLE, NULL, working_directory, &info, &processInfo))
     {
         gchar* stdout_buffer = (gchar*)g_malloc(BUFFER_SIZE);
         gchar* stderr_buffer = (gchar*)g_malloc(BUFFER_SIZE);
@@ -528,6 +533,7 @@ GPid ws_pipe_spawn_async(ws_pipe_t *ws_pipe, GPtrArray *args)
     HANDLE child_stdout_wr = NULL;
     HANDLE child_stderr_rd = NULL;
     HANDLE child_stderr_wr = NULL;
+    HANDLE inherit_handles[3];
 #endif
 
     // XXX harmonize handling of command arguments for the sync/async functions
@@ -540,7 +546,7 @@ GPid ws_pipe_spawn_async(ws_pipe_t *ws_pipe, GPtrArray *args)
 
 #ifdef _WIN32
     sa.nLength = sizeof(SECURITY_ATTRIBUTES);
-    sa.bInheritHandle = TRUE;
+    sa.bInheritHandle = FALSE;
     sa.lpSecurityDescriptor = NULL;
 
     if (!CreatePipe(&child_stdin_rd, &child_stdin_wr, &sa, 0))
@@ -573,6 +579,10 @@ GPid ws_pipe_spawn_async(ws_pipe_t *ws_pipe, GPtrArray *args)
         return WS_INVALID_PID;
     }
 
+    inherit_handles[0] = child_stdin_rd;
+    inherit_handles[1] = child_stderr_wr;
+    inherit_handles[2] = child_stdout_wr;
+
     memset(&processInfo, 0, sizeof(PROCESS_INFORMATION));
     memset(&info, 0, sizeof(STARTUPINFO));
 
@@ -583,7 +593,8 @@ GPid ws_pipe_spawn_async(ws_pipe_t *ws_pipe, GPtrArray *args)
     info.dwFlags = STARTF_USESTDHANDLES | STARTF_USESHOWWINDOW;
     info.wShowWindow = SW_HIDE;
 
-    if (win32_create_process(NULL, command_line, NULL, NULL, TRUE, CREATE_NEW_CONSOLE, NULL, NULL, &info, &processInfo))
+    if (win32_create_process(NULL, command_line, NULL, NULL, G_N_ELEMENTS(inherit_handles), inherit_handles,
+                             CREATE_NEW_CONSOLE, NULL, NULL, &info, &processInfo))
     {
         stdin_fd = _open_osfhandle((intptr_t)(child_stdin_wr), _O_BINARY);
         stdout_fd = _open_osfhandle((intptr_t)(child_stdout_rd), _O_BINARY);