Bug 1276386 - Prevent processes from inheriting extra file descriptors on Windows. r=mhowell

MozReview-Commit-ID: IFi2Z7sqaxW

--HG--
extra : rebase_source : fc8c270847e29a88c6b27c3b18bf39f8eb1f03fd

toolkit/modules/subprocess/subprocess_shared_win.js

@@ -13,10 +13,12 @@ const Win = OS.Constants.Win;
 
 const LIBC_CHOICES = ["kernel32.dll"];
 
-const win32 = {
+var win32 = {
   // On Windows 64, winapi_abi is an alias for default_abi.
   WINAPI: ctypes.winapi_abi,
 
+  VOID: ctypes.void_t,
+
   BYTE: ctypes.uint8_t,
   WORD: ctypes.uint16_t,
   DWORD: ctypes.uint32_t,
@@ -34,15 +36,24 @@ const win32 = {
   WCHAR: ctypes.jschar,
 
   ULONG_PTR: ctypes.uintptr_t,
+
+  SIZE_T: ctypes.size_t,
+  PSIZE_T: ctypes.size_t.ptr,
 };
 
 Object.assign(win32, {
+  DWORD_PTR: win32.ULONG_PTR,
+
   LPSTR: win32.CHAR.ptr,
   LPWSTR: win32.WCHAR.ptr,
 
   LPBYTE: win32.BYTE.ptr,
   LPDWORD: win32.DWORD.ptr,
   LPHANDLE: win32.HANDLE.ptr,
+
+  // This is an opaque type.
+  PROC_THREAD_ATTRIBUTE_LIST: ctypes.char.array(),
+  LPPROC_THREAD_ATTRIBUTE_LIST: ctypes.char.ptr,
 });
 
 Object.assign(win32, {
@@ -54,6 +65,7 @@ Object.assign(win32, {
 Object.assign(win32, {
   CREATE_NEW_CONSOLE: 0x00000010,
   CREATE_UNICODE_ENVIRONMENT: 0x00000400,
+  EXTENDED_STARTUPINFO_PRESENT: 0x00080000,
   CREATE_NO_WINDOW: 0x08000000,
 
   STARTF_USESTDHANDLES: 0x0100,
@@ -63,6 +75,7 @@ Object.assign(win32, {
 
   ERROR_HANDLE_EOF: 38,
   ERROR_BROKEN_PIPE: 109,
+  ERROR_INSUFFICIENT_BUFFER: 122,
 
   FILE_FLAG_OVERLAPPED: 0x40000000,
 
@@ -77,6 +90,8 @@ Object.assign(win32, {
 
   STILL_ACTIVE: 259,
 
+  PROC_THREAD_ATTRIBUTE_HANDLE_LIST: 0x00020002,
+
   // These constants are 32-bit unsigned integers, but Windows defines
   // them as negative integers cast to an unsigned type.
   STD_INPUT_HANDLE: -10 + 0x100000000,
@@ -131,6 +146,14 @@ Object.assign(win32, {
   ]),
 });
 
+Object.assign(win32, {
+  STARTUPINFOEXW: new ctypes.StructType("STARTUPINFOEXW", [
+    {"StartupInfo": win32.STARTUPINFOW},
+    {"lpAttributeList": win32.LPPROC_THREAD_ATTRIBUTE_LIST},
+  ]),
+});
+
+
 var libc = new Library("libc", LIBC_CHOICES, {
   CloseHandle: [
     win32.WINAPI,
@@ -196,6 +219,12 @@ var libc = new Library("libc", LIBC_CHOICES, {
     win32.PROCESS_INFORMATION.ptr, /* out lpProcessInformation */
   ],
 
+  DeleteProcThreadAttributeList: [
+    win32.WINAPI,
+    win32.VOID,
+    win32.LPPROC_THREAD_ATTRIBUTE_LIST, /* in/out lpAttributeList */
+  ],
+
   DuplicateHandle: [
     win32.WINAPI,
     win32.BOOL,
@@ -251,6 +280,15 @@ var libc = new Library("libc", LIBC_CHOICES, {
     win32.DWORD, /* nStdHandle */
   ],
 
+  InitializeProcThreadAttributeList: [
+    win32.WINAPI,
+    win32.BOOL,
+    win32.LPPROC_THREAD_ATTRIBUTE_LIST, /* out opt lpAttributeList */
+    win32.DWORD, /* dwAttributeCount */
+    win32.DWORD, /* dwFlags */
+    win32.PSIZE_T, /* in/out lpSize */
+  ],
+
   ReadFile: [
     win32.WINAPI,
     win32.BOOL,
@@ -268,6 +306,18 @@ var libc = new Library("libc", LIBC_CHOICES, {
     win32.UINT, /* uExitCode */
   ],
 
+  UpdateProcThreadAttribute: [
+    win32.WINAPI,
+    win32.BOOL,
+    win32.LPPROC_THREAD_ATTRIBUTE_LIST, /* in/out lpAttributeList */
+    win32.DWORD, /* dwFlags */
+    win32.DWORD_PTR, /* Attribute */
+    win32.PVOID, /* lpValue */
+    win32.SIZE_T, /* cbSize */
+    win32.PVOID, /* out opt lpPreviousValue */
+    win32.PSIZE_T, /* opt lpReturnSize */
+  ],
+
   WaitForMultipleObjects: [
     win32.WINAPI,
     win32.DWORD,
@@ -341,3 +391,37 @@ win32.createPipe = function(secAttr, readFlags = 0, writeFlags = 0, size = 0) {
   return [win32.Handle(readHandle),
           win32.Handle(writeHandle)];
 };
+
+win32.createThreadAttributeList = function(handles) {
+  try {
+    void libc.InitializeProcThreadAttributeList;
+    void libc.DeleteProcThreadAttributeList;
+    void libc.UpdateProcThreadAttribute;
+  } catch (e) {
+    // This is only supported in Windows Vista and later.
+    return null;
+  }
+
+  let size = win32.SIZE_T();
+  if (!libc.InitializeProcThreadAttributeList(null, 1, 0, size.address()) &&
+      ctypes.winLastError != win32.ERROR_INSUFFICIENT_BUFFER) {
+    return null;
+  }
+
+  let attrList = win32.PROC_THREAD_ATTRIBUTE_LIST(size.value);
+
+  if (!libc.InitializeProcThreadAttributeList(attrList, 1, 0, size.address())) {
+    return null;
+  }
+
+  let ok = libc.UpdateProcThreadAttribute(
+    attrList, 0, win32.PROC_THREAD_ATTRIBUTE_HANDLE_LIST,
+    handles, handles.constructor.size, null, null);
+
+  if (!ok) {
+    libc.DeleteProcThreadAttributeList(attrList);
+    return null;
+  }
+
+  return attrList;
+};

toolkit/modules/subprocess/subprocess_worker_win.js

@@ -425,7 +425,9 @@ class Process extends BaseProcess {
     let processFlags = win32.CREATE_NO_WINDOW
                      | win32.CREATE_UNICODE_ENVIRONMENT;
 
-    let startupInfo = new win32.STARTUPINFOW();
+    let startupInfoEx = new win32.STARTUPINFOEXW();
+    let startupInfo = startupInfoEx.StartupInfo;
+
     startupInfo.cb = win32.STARTUPINFOW.size;
     startupInfo.dwFlags = win32.STARTF_USESTDHANDLES;
 
@@ -433,6 +435,19 @@ class Process extends BaseProcess {
     startupInfo.hStdOutput = handles[1];
     startupInfo.hStdError = handles[2];
 
+    // Note: This needs to be kept alive until we destroy the attribute list.
+    let handleArray = win32.HANDLE.array()(handles);
+
+    let threadAttrs = win32.createThreadAttributeList(handleArray);
+    if (threadAttrs) {
+      // If have thread attributes to pass, pass the size of the full extended
+      // startup info struct.
+      processFlags |= win32.EXTENDED_STARTUPINFO_PRESENT;
+      startupInfo.cb = win32.STARTUPINFOEXW.size;
+
+      startupInfoEx.lpAttributeList = threadAttrs;
+    }
+
     let procInfo = new win32.PROCESS_INFORMATION();
 
     let ok = libc.CreateProcessW(
@@ -448,6 +463,10 @@ class Process extends BaseProcess {
       handle.dispose();
     }
 
+    if (threadAttrs) {
+      libc.DeleteProcThreadAttributeList(threadAttrs);
+    }
+
     if (!ok) {
       for (let pipe of this.pipes) {
         pipe.close();

toolkit/modules/subprocess/test/xpcshell/test_subprocess.js

@@ -413,6 +413,44 @@ add_task(function* test_subprocess_invalid_json() {
 });
 
 
+if (AppConstants.isPlatformAndVersionAtLeast("win", "6")) {
+  add_task(function* test_subprocess_inherited_descriptors() {
+    let {ctypes, libc, win32} = Cu.import("resource://gre/modules/subprocess/subprocess_win.jsm");
+
+    let secAttr = new win32.SECURITY_ATTRIBUTES();
+    secAttr.nLength = win32.SECURITY_ATTRIBUTES.size;
+    secAttr.bInheritHandle = true;
+
+    let handles = win32.createPipe(secAttr, 0);
+
+
+    let proc = yield Subprocess.call({
+      command: PYTHON,
+      arguments: ["-u", TEST_SCRIPT, "echo"],
+    });
+
+
+    // Close the output end of the pipe.
+    // Ours should be the only copy, so reads should fail after this.
+    handles[1].dispose();
+
+    let buffer = new ArrayBuffer(1);
+    let succeeded = libc.ReadFile(handles[0], buffer, buffer.byteLength,
+                                  null, null);
+
+    ok(!succeeded, "ReadFile should fail on broken pipe");
+    equal(ctypes.winLastError, win32.ERROR_BROKEN_PIPE, "Read should fail with ERROR_BROKEN_PIPE");
+
+
+    proc.stdin.close();
+
+    let {exitCode} = yield proc.wait();
+
+    equal(exitCode, 0, "Got expected exit code");
+  });
+}
+
+
 add_task(function* test_subprocess_wait() {
   let proc = yield Subprocess.call({
     command: PYTHON,