Restrict Windows subprocess handle inheritance

subprocess.h

@@ -351,6 +351,7 @@ typedef struct _PROCESS_INFORMATION *LPPROCESS_INFORMATION;
 typedef struct _SECURITY_ATTRIBUTES *LPSECURITY_ATTRIBUTES;
 typedef struct _STARTUPINFOW *LPSTARTUPINFOW;
 typedef struct _OVERLAPPED *LPOVERLAPPED;
+typedef struct _PROC_THREAD_ATTRIBUTE_LIST *LPPROC_THREAD_ATTRIBUTE_LIST;
 
 #ifdef __clang__
 #pragma clang diagnostic pop
@@ -402,6 +403,11 @@ struct subprocess_startup_info_s {
   void *hStdError;
 };
 
+struct subprocess_startup_info_ex_s {
+  struct subprocess_startup_info_s startupInfo;
+  void *attributeList;
+};
+
 struct subprocess_overlapped_s {
   uintptr_t Internal;
   uintptr_t InternalHigh;
@@ -451,6 +457,14 @@ __declspec(dllimport) int __stdcall CreateProcessW(
     const subprocess_wchar_t *, subprocess_wchar_t *, LPSECURITY_ATTRIBUTES,
     LPSECURITY_ATTRIBUTES, int, unsigned long, void *,
     const subprocess_wchar_t *, LPSTARTUPINFOW, LPPROCESS_INFORMATION);
+__declspec(dllimport) int __stdcall
+InitializeProcThreadAttributeList(LPPROC_THREAD_ATTRIBUTE_LIST, unsigned long,
+                                  unsigned long, subprocess_size_t *);
+__declspec(dllimport) int __stdcall UpdateProcThreadAttribute(
+    LPPROC_THREAD_ATTRIBUTE_LIST, unsigned long, subprocess_size_t, void *,
+    subprocess_size_t, void *, subprocess_size_t *);
+__declspec(dllimport) void __stdcall
+DeleteProcThreadAttributeList(LPPROC_THREAD_ATTRIBUTE_LIST);
 __declspec(dllimport) int __stdcall MultiByteToWideChar(
     unsigned int, unsigned long, const char *, int, subprocess_wchar_t *, int);
 __declspec(dllimport) int __stdcall CloseHandle(void *);
@@ -735,20 +749,28 @@ int subprocess_create_ex(const char *const commandLine[], int options,
   subprocess_size_t bs_run;
   unsigned long flags = 0;
   unsigned long last_error = 0;
+  int attribute_list_initialized = 0;
   int result = subprocess_error_unknown;
   const unsigned int codePageUtf8 = 65001;
   const unsigned long mbErrInvalidChars = 0x00000008;
   const unsigned long startFUseStdHandles = 0x00000100;
   const unsigned long handleFlagInherit = 0x00000001;
   const unsigned long createNoWindow = 0x08000000;
   const unsigned long createUnicodeEnvironment = 0x00000400;
+  const unsigned long extendedStartupInfoPresent = 0x00080000;
+  const subprocess_size_t procThreadAttributeHandleList = 0x00020002;
   struct subprocess_subprocess_information_s processInfo = {SUBPROCESS_NULL,
                                                             SUBPROCESS_NULL, 0,
                                                             0};
   struct subprocess_security_attributes_s saAttr = {sizeof(saAttr),
                                                     SUBPROCESS_NULL, 1};
   subprocess_wchar_t empty_environment[2] = {0, 0};
   subprocess_wchar_t *used_environment = SUBPROCESS_NULL;
+  subprocess_size_t attribute_list_size = 0;
+  subprocess_size_t inherited_handle_count = 0;
+  LPPROC_THREAD_ATTRIBUTE_LIST attribute_list = SUBPROCESS_NULL;
+  void *inherited_handles[3];
+  struct subprocess_startup_info_ex_s startInfoEx;
   struct subprocess_startup_info_s startInfo = {0,
                                                 SUBPROCESS_NULL,
                                                 SUBPROCESS_NULL,
@@ -1123,6 +1145,44 @@ int subprocess_create_ex(const char *const commandLine[], int options,
     }
   }
 
+  /* Restrict inheritance to this subprocess's standard streams. Without a
+     handle list, concurrent subprocess_create calls can inherit each other's
+     temporarily-inheritable child pipe handles. */
+  inherited_handles[inherited_handle_count++] = startInfo.hStdInput;
+  inherited_handles[inherited_handle_count++] = startInfo.hStdOutput;
+  if (startInfo.hStdError != startInfo.hStdOutput) {
+    inherited_handles[inherited_handle_count++] = startInfo.hStdError;
+  }
+
+  InitializeProcThreadAttributeList(SUBPROCESS_NULL, 1, 0,
+                                    &attribute_list_size);
+  if (0 == attribute_list_size) {
+    result = subprocess_error_spawn;
+    goto cleanup;
+  }
+
+  attribute_list = SUBPROCESS_PTR_CAST(LPPROC_THREAD_ATTRIBUTE_LIST,
+                                       _alloca(attribute_list_size));
+  if (!attribute_list || !InitializeProcThreadAttributeList(
+                             attribute_list, 1, 0, &attribute_list_size)) {
+    result = subprocess_error_spawn;
+    goto cleanup;
+  }
+  attribute_list_initialized = 1;
+
+  if (!UpdateProcThreadAttribute(
+          attribute_list, 0, procThreadAttributeHandleList, inherited_handles,
+          inherited_handle_count * sizeof(inherited_handles[0]),
+          SUBPROCESS_NULL, SUBPROCESS_NULL)) {
+    result = subprocess_error_spawn;
+    goto cleanup;
+  }
+
+  startInfoEx.startupInfo = startInfo;
+  startInfoEx.startupInfo.cb = sizeof(startInfoEx);
+  startInfoEx.attributeList = attribute_list;
+  flags |= extendedStartupInfoPresent;
+
   if (!CreateProcessW(
           SUBPROCESS_NULL,
           commandLineCombinedWide, // command line
@@ -1133,7 +1193,7 @@ int subprocess_create_ex(const char *const commandLine[], int options,
           used_environment,        // used environment
           process_cwd_wide,        // use specified current directory
           SUBPROCESS_PTR_CAST(LPSTARTUPINFOW,
-                              &startInfo), // STARTUPINFO pointer
+                              &startInfoEx), // STARTUPINFOEX pointer
           SUBPROCESS_PTR_CAST(LPPROCESS_INFORMATION, &processInfo))) {
     result = subprocess_error_from_windows_error(GetLastError());
     if (subprocess_error_unknown == result) {
@@ -1142,6 +1202,9 @@ int subprocess_create_ex(const char *const commandLine[], int options,
     goto cleanup;
   }
 
+  DeleteProcThreadAttributeList(attribute_list);
+  attribute_list_initialized = 0;
+
   out_process->hProcess = processInfo.hProcess;
   processInfo.hProcess = SUBPROCESS_NULL;
 
@@ -1171,6 +1234,10 @@ int subprocess_create_ex(const char *const commandLine[], int options,
 cleanup:
   last_error = GetLastError();
 
+  if (attribute_list_initialized) {
+    DeleteProcThreadAttributeList(attribute_list);
+  }
+
   if (subprocess_error_unknown == result) {
     result = subprocess_error_from_windows_error(last_error);
   }

test/CMakeLists.txt

@@ -63,6 +63,7 @@ set(SUBPROCESS_HELPER_SOURCES
   process_call_return_argc.c
   process_cwd.c
   process_is_fd_open.c
+  process_signal_handle.c
 )
 
 foreach(SUBPROCESS_HELPER_SOURCE ${SUBPROCESS_HELPER_SOURCES})

test/process_signal_handle.c

@@ -0,0 +1,26 @@
+#include <stdio.h>
+
+#ifdef _MSC_VER
+#pragma warning(disable : 4996)
+#endif
+
+#if defined(_WIN32)
+__declspec(dllimport) int __stdcall SetEvent(void *);
+#endif
+
+int main(int argc, char *argv[]) {
+#if defined(_WIN32)
+  void *handle = 0;
+
+  if ((2 != argc) || (1 != sscanf(argv[1], "%p", &handle))) {
+    return 2;
+  }
+
+  SetEvent(handle);
+  return 0;
+#else
+  (void)argc;
+  (void)argv;
+  return 0;
+#endif
+}

test/test.c

@@ -38,3 +38,35 @@ UTEST(c, create_does_not_inherit_another_subprocess_pipe) {
                 "the second subprocess inherited the first subprocess pipe");
 #endif
 }
+
+#if defined(_WIN32)
+UTEST(c, create_does_not_inherit_unlisted_windows_handle) {
+  const unsigned long wait_timeout = 0x00000102;
+  const char *command_line[] = {"./process_signal_handle", 0, 0};
+  struct subprocess_security_attributes_s security_attributes = {
+      sizeof(security_attributes), SUBPROCESS_NULL, 1};
+  struct subprocess_s process;
+  char handle_argument[32];
+  void *inheritable_handle;
+  unsigned long wait_result;
+  int return_code = -1;
+
+  inheritable_handle = CreateEventA(
+      SUBPROCESS_PTR_CAST(LPSECURITY_ATTRIBUTES, &security_attributes), 1, 0,
+      SUBPROCESS_NULL);
+  ASSERT_TRUE(inheritable_handle);
+  ASSERT_TRUE(0 < snprintf(handle_argument, sizeof(handle_argument), "%p",
+                           inheritable_handle));
+  command_line[1] = handle_argument;
+
+  ASSERT_EQ(0, subprocess_create(command_line, 0, &process));
+  ASSERT_EQ(0, subprocess_join(&process, &return_code));
+  ASSERT_EQ(0, subprocess_destroy(&process));
+  wait_result = WaitForSingleObject(inheritable_handle, 0);
+  ASSERT_TRUE(CloseHandle(inheritable_handle));
+
+  EXPECT_EQ(0, return_code);
+  EXPECT_EQ_MSG(wait_timeout, wait_result,
+                "subprocess inherited a handle outside its standard streams");
+}
+#endif