Cygwin: fork/exec: fix child process permissions

- Exec'ed/spawned processes don't need PROCESS_DUP_HANDLE.  Remove that
  permission from the parent handle.

- PROCESS_QUERY_LIMITED_INFORMATION doesn't work for Windows 7 if the
  process is started as a service.  Add PROCESS_QUERY_INFORMATION for
  pre-Windows 8 in that case.

Signed-off-by: Corinna Vinschen <corinna@vinschen.de>

5 files changed, 38 insertions, 8 deletions

diff --git a/winsup/cygwin/security.h b/winsup/cygwin/security.h
index 70912b4fc..ec68171a3 100644
--- a/winsup/cygwin/security.h
+++ b/winsup/cygwin/security.h
@@ -17,6 +17,7 @@ details. */
 
 /* UID/GID */
 void uinfo_init ();
+bool check_token_membership (HANDLE, PSID);
 bool check_token_membership (PSID);
 
 #define ILLEGAL_UID ((uid_t)-1)
diff --git a/winsup/cygwin/sigproc.cc b/winsup/cygwin/sigproc.cc
index 45e948251..42eeb304d 100644
--- a/winsup/cygwin/sigproc.cc
+++ b/winsup/cygwin/sigproc.cc
@@ -811,12 +811,24 @@ child_info::child_info (unsigned in_cb, child_info_types chtype,
     }
   sigproc_printf ("subproc_ready %p", subproc_ready);
   /* Create an inheritable handle to pass to the child process.  This will
-     allow the child to duplicate handles from the parent to itself. */
+     allow the child to copy cygheap etc. from the parent to itself.  If
+     we're forking, we also need handle duplicate access. */
   parent = NULL;
+  DWORD perms = PROCESS_QUERY_LIMITED_INFORMATION | PROCESS_VM_READ;
+  if (type == _CH_FORK)
+    {
+      perms |= PROCESS_DUP_HANDLE;
+      /* For some reason fork on Windows 7 requires PROCESS_QUERY_INFORMATION
+	 rather than just PROCESS_QUERY_LIMITED_INFORMATION when started as a
+	 service. */
+      if (wincap.needs_query_information ()
+	  && (cygheap->user.saved_sid () == well_known_system_sid
+	      || check_token_membership (hProcToken, well_known_service_sid)))
+	perms |= PROCESS_QUERY_INFORMATION;
+    }
+
   if (!DuplicateHandle (GetCurrentProcess (), GetCurrentProcess (),
-			GetCurrentProcess (), &parent,
-			PROCESS_DUP_HANDLE | PROCESS_VM_READ
-			| PROCESS_QUERY_LIMITED_INFORMATION, TRUE, 0))
+			GetCurrentProcess (), &parent, perms, TRUE, 0))
     system_printf ("couldn't create handle to myself for child, %E");
 }
 
diff --git a/winsup/cygwin/uinfo.cc b/winsup/cygwin/uinfo.cc
index 8dcf731de..00a2b5abb 100644
--- a/winsup/cygwin/uinfo.cc
+++ b/winsup/cygwin/uinfo.cc
@@ -118,16 +118,13 @@ cygheap_user::init ()
    This needs careful checking should we use check_token_membership in other
    circumstances. */
 bool
-check_token_membership (PSID sid)
+check_token_membership (HANDLE tok, PSID sid)
 {
   NTSTATUS status;
   ULONG size;
   tmp_pathbuf tp;
   PTOKEN_GROUPS groups = (PTOKEN_GROUPS) tp.w_get ();
 
-  /* If impersonated, use impersonation token. */
-  HANDLE tok = cygheap->user.issetuid () ? cygheap->user.primary_token ()
-					 : hProcToken;
   status = NtQueryInformationToken (tok, TokenGroups, groups, 2 * NT_MAX_PATH,
 				    &size);
   if (!NT_SUCCESS (status))
@@ -142,6 +139,15 @@ check_token_membership (PSID sid)
   return false;
 }
 
+bool
+check_token_membership (PSID sid)
+{
+  /* If impersonated, use impersonation token. */
+  HANDLE tok = cygheap->user.issetuid () ? cygheap->user.primary_token ()
+					 : hProcToken;
+  return check_token_membership (tok, sid);
+}
+
 static void
 internal_getlogin (cygheap_user &user)
 {
diff --git a/winsup/cygwin/wincap.cc b/winsup/cygwin/wincap.cc
index 4c1c7b401..132265c6a 100644
--- a/winsup/cygwin/wincap.cc
+++ b/winsup/cygwin/wincap.cc
@@ -23,6 +23,7 @@ wincaps wincap_vista __attribute__((section (".cygwin_dll_common"), shared)) = {
   {
     is_server:false,
     needs_count_in_si_lpres2:true,
+    needs_query_information:true,
     has_gaa_largeaddress_bug:true,
     has_broken_alloc_console:false,
     has_console_logon_sid:false,
@@ -46,6 +47,7 @@ wincaps wincap_7 __attribute__((section (".cygwin_dll_common"), shared)) = {
   {
     is_server:false,
     needs_count_in_si_lpres2:false,
+    needs_query_information:true,
     has_gaa_largeaddress_bug:true,
     has_broken_alloc_console:true,
     has_console_logon_sid:true,
@@ -69,6 +71,7 @@ wincaps wincap_8 __attribute__((section (".cygwin_dll_common"), shared)) = {
   {
     is_server:false,
     needs_count_in_si_lpres2:false,
+    needs_query_information:false,
     has_gaa_largeaddress_bug:false,
     has_broken_alloc_console:true,
     has_console_logon_sid:true,
@@ -92,6 +95,7 @@ wincaps  wincap_10_1507 __attribute__((section (".cygwin_dll_common"), shared))
   {
     is_server:false,
     needs_count_in_si_lpres2:false,
+    needs_query_information:false,
     has_gaa_largeaddress_bug:false,
     has_broken_alloc_console:true,
     has_console_logon_sid:true,
@@ -115,6 +119,7 @@ wincaps wincap_10_1511 __attribute__((section (".cygwin_dll_common"), shared)) =
   {
     is_server:false,
     needs_count_in_si_lpres2:false,
+    needs_query_information:false,
     has_gaa_largeaddress_bug:false,
     has_broken_alloc_console:true,
     has_console_logon_sid:true,
@@ -138,6 +143,7 @@ wincaps wincap_10_1703 __attribute__((section (".cygwin_dll_common"), shared)) =
   {
     is_server:false,
     needs_count_in_si_lpres2:false,
+    needs_query_information:false,
     has_gaa_largeaddress_bug:false,
     has_broken_alloc_console:true,
     has_console_logon_sid:true,
@@ -161,6 +167,7 @@ wincaps wincap_10_1709 __attribute__((section (".cygwin_dll_common"), shared)) =
   {
     is_server:false,
     needs_count_in_si_lpres2:false,
+    needs_query_information:false,
     has_gaa_largeaddress_bug:false,
     has_broken_alloc_console:true,
     has_console_logon_sid:true,
@@ -184,6 +191,7 @@ wincaps wincap_10_1803 __attribute__((section (".cygwin_dll_common"), shared)) =
   {
     is_server:false,
     needs_count_in_si_lpres2:false,
+    needs_query_information:false,
     has_gaa_largeaddress_bug:false,
     has_broken_alloc_console:true,
     has_console_logon_sid:true,
@@ -207,6 +215,7 @@ wincaps wincap_10_1809 __attribute__((section (".cygwin_dll_common"), shared)) =
   {
     is_server:false,
     needs_count_in_si_lpres2:false,
+    needs_query_information:false,
     has_gaa_largeaddress_bug:false,
     has_broken_alloc_console:true,
     has_console_logon_sid:true,
diff --git a/winsup/cygwin/wincap.h b/winsup/cygwin/wincap.h
index f8846f91d..75b7a9f0a 100644
--- a/winsup/cygwin/wincap.h
+++ b/winsup/cygwin/wincap.h
@@ -17,6 +17,7 @@ struct wincaps
   struct  __attribute__ ((aligned (8))) {
     unsigned is_server				: 1;
     unsigned needs_count_in_si_lpres2		: 1;
+    unsigned needs_query_information		: 1;
     unsigned has_gaa_largeaddress_bug		: 1;
     unsigned has_broken_alloc_console		: 1;
     unsigned has_console_logon_sid		: 1;
@@ -70,6 +71,7 @@ public:
   }
   bool  IMPLEMENT (is_server)
   bool	IMPLEMENT (needs_count_in_si_lpres2)
+  bool	IMPLEMENT (needs_query_information)
   bool	IMPLEMENT (has_gaa_largeaddress_bug)
   bool	IMPLEMENT (has_broken_alloc_console)
   bool	IMPLEMENT (has_console_logon_sid)