This is the mail archive of the mailing list for the Cygwin project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: More security issues

On Sun, Feb 10, 2002 at 02:34:55PM -0500, Pierre A. Humblet wrote:
>  I wonder what the sa in CreateProcess
> really does... The only thing that has an effect is the Inherit flag.

MSDN documents the SD in the lpProcessAttributes/lpThreadAttributes
argument being used as the SD of the called process/main thread.
The SD of the process seems not to correspond with the default DACL
in the token.  However, the sec_user() isn't w/o effect.  You
can easily check that by changing the function to create a wrong

> In the course of debugging I also noticed that the sid2 passed
> to sec_user() from just before CreateProcessAsUser() is useless.
> It is actually equal to the sid that sec_user() gets from 
> cygheap->user.sid ()  [cygheap->user is set in seteuid()]

Does the following patch help?

RCS file: /cvs/src/src/winsup/cygwin/,v
retrieving revision 1.97
diff -u -p -r1.97
---	2002/02/10 13:38:49	1.97
+++	2002/02/12 15:54:53
@@ -647,6 +647,11 @@ spawn_guts (HANDLE hToken, const char * 
+      /* Remove impersonation */
+      if (cygheap->user.impersonated
+	  && cygheap->user.token != INVALID_HANDLE_VALUE)
+	RevertToSelf ();
       cygsid sid;
       DWORD ret_len;
       if (!GetTokenInformation (hToken, TokenUser, &sid, sizeof sid, &ret_len))
@@ -659,11 +664,6 @@ spawn_guts (HANDLE hToken, const char * 
       PSECURITY_ATTRIBUTES sec_attribs = allow_ntsec && sid
 					 ? sec_user (sa_buf, sid)
 					 : &sec_all_nih;
-      /* Remove impersonation */
-      if (cygheap->user.impersonated
-	  && cygheap->user.token != INVALID_HANDLE_VALUE)
-	RevertToSelf ();
       static BOOL first_time = TRUE;
       if (first_time)

> All of this effort was motivated by weird access issues to the 
> impersonation token. I can fix that by opening the thread token
> security descriptor after ImpersonateLoggedOnUser() in seteuid()
> and changing the ACL (using the ACL from sec_user(), that works!). 
> Unfortunately the work must be redone each time the sequence 
> RevertToSelf(), ..., ImpersonateLoggedOnUser() occurs. 

That can't be the way to go.  Somehow we should try to figure out to do
it correctly.

> Back to setegid(), another safe way would be to 
> RevertToSelf(),..,Impersonate..() if currently impersonated.
> That's because there is also a RevertToSelf() before CreateProcessAsUser()
> Why is there one, by the way? Microsoft seems to suggest working in the
> security context of the new user. It says it's useful if the executable 
> is only executable by the new user.

Did you try if that works reliable?  Nobody keeps you from patching it ;-)


Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Developer                      
Red Hat, Inc.

Unsubscribe info:
Bug reporting:

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]