From da5b48ef3c39ea68d41a43cfc4379124b67c0812 Mon Sep 17 00:00:00 2001 From: Corinna Vinschen Date: Sat, 25 Jun 2016 00:42:36 +0200 Subject: [PATCH] No longer support "Interact with desktop" Always create child user window station and desktop, unless only spawning with restricted token. Also fix formatting of a few comments in child_info_spawn::worker. Signed-off-by: Corinna Vinschen --- winsup/cygwin/spawn.cc | 48 ++++++++++++++++++------------------------ 1 file changed, 20 insertions(+), 28 deletions(-) diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc index 1c4f1f502..37db52608 100644 --- a/winsup/cygwin/spawn.cc +++ b/winsup/cygwin/spawn.cc @@ -602,31 +602,22 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv, if (mode == _P_OVERLAY) myself.set_acl(); - WCHAR wstname[1024] = { L'\0' }; - HWINSTA hwst_orig = NULL, hwst = NULL; - HDESK hdsk_orig = NULL, hdsk = NULL; - PSECURITY_ATTRIBUTES sa; - DWORD n; - - hwst_orig = GetProcessWindowStation (); - hdsk_orig = GetThreadDesktop (GetCurrentThreadId ()); - GetUserObjectInformationW (hwst_orig, UOI_NAME, wstname, 1024, &n); - /* Prior to Vista it was possible to start a service with the - "Interact with desktop" flag. This started the service in the - interactive window station of the console. A big security - risk, but we don't want to disable this behaviour for older - OSes because it's still heavily used by some users. They have - been warned. */ - if (!::cygheap->user.setuid_to_restricted - && wcscasecmp (wstname, L"WinSta0") != 0) + HWINSTA hwst = NULL; + HWINSTA hwst_orig = GetProcessWindowStation (); + HDESK hdsk = NULL; + HDESK hdsk_orig = GetThreadDesktop (GetCurrentThreadId ()); + /* Don't create WindowStation and Desktop for restricted child. */ + if (!::cygheap->user.setuid_to_restricted) { + PSECURITY_ATTRIBUTES sa; WCHAR sid[128]; + WCHAR wstname[1024] = { L'\0' }; sa = sec_user ((PSECURITY_ATTRIBUTES) alloca (1024), ::cygheap->user.sid ()); - /* We're creating a window station per user, not per logon session. - First of all we might not have a valid logon session for - the user (logon by create_token), and second, it doesn't + /* We're creating a window station per user, not per logon + session First of all we might not have a valid logon session + for the user (logon by create_token), and second, it doesn't make sense in terms of security to create a new window station for every logon of the same user. It just fills up the system with window stations for no good reason. */ @@ -688,15 +679,16 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv, myself->exec_sendsig = NULL; } myself->process_state &= ~PID_NOTCYGWIN; - /* Reset handle inheritance to default when the execution of a non-Cygwin - process fails. Only need to do this for _P_OVERLAY since the handle will - be closed otherwise. Don't need to do this for 'parent' since it will - be closed in every case. See FIXME above. */ + /* Reset handle inheritance to default when the execution of a' + non-Cygwin process fails. Only need to do this for _P_OVERLAY + since the handle will be closed otherwise. Don't need to do + this for 'parent' since it will be closed in every case. + See FIXME above. */ if (!iscygwin () && mode == _P_OVERLAY) SetHandleInformation (wr_proc_pipe, HANDLE_FLAG_INHERIT, HANDLE_FLAG_INHERIT); if (wr_proc_pipe == my_wr_proc_pipe) - wr_proc_pipe = NULL; /* We still own it: don't nuke in destructor */ + wr_proc_pipe = NULL; /* We still own it: don't nuke in destructor */ /* Restore impersonation. In case of _P_OVERLAY this isn't allowed since it would overwrite child data. */ @@ -721,7 +713,7 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv, else cygpid = myself->pid; - /* We print the original program name here so the user can see that too. */ + /* Print the original program name here so the user can see that too. */ syscall_printf ("pid %d, prog_arg %s, cmd line %.9500s)", rc ? cygpid : (unsigned int) -1, prog_arg, (const char *) cmd); @@ -758,8 +750,8 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv, real_path.get_wide_win32_path (child->progname); /* FIXME: This introduces an unreferenced, open handle into the child. The purpose is to keep the pid shared memory open so that all of - the fields filled out by child.remember do not disappear and so there - is not a brief period during which the pid is not available. + the fields filled out by child.remember do not disappear and so + there is not a brief period during which the pid is not available. However, we should try to find another way to do this eventually. */ DuplicateHandle (GetCurrentProcess (), child.shared_handle (), pi.hProcess, NULL, 0, 0, DUPLICATE_SAME_ACCESS);