[newlib-cygwin] Cygwin: fork/exec: fix child process permissions
Corinna Vinschen
corinna@sourceware.org
Tue Jan 29 17:00:00 GMT 2019
https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=5a0f2c00aa019de73a6077ca3017b594c43184a4
commit 5a0f2c00aa019de73a6077ca3017b594c43184a4
Author: Corinna Vinschen <corinna@vinschen.de>
Date: Tue Jan 29 16:26:45 2019 +0100
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>
Diff:
---
winsup/cygwin/security.h | 1 +
winsup/cygwin/sigproc.cc | 20 ++++++++++++++++----
winsup/cygwin/uinfo.cc | 14 ++++++++++----
winsup/cygwin/wincap.cc | 9 +++++++++
winsup/cygwin/wincap.h | 2 ++
5 files changed, 38 insertions(+), 8 deletions(-)
diff --git a/winsup/cygwin/security.h b/winsup/cygwin/security.h
index 70912b4..ec68171 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 45e9482..42eeb30 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 8dcf731..00a2b5a 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 4c1c7b4..132265c 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 f8846f9..75b7a9f 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)
More information about the Cygwin-cvs
mailing list