Windows username in get_group_sidlist

Pierre A. Humblet Pierre.Humblet@ieee.org
Sun Jun 30 05:57:00 GMT 2002


At 11:25 AM 6/25/2002 +0200, Corinna Vinschen wrote:
>> I would actually read passwd by calling extract_nt_dom_user (),
>> modifying it to first read the domain from the passwd file, and 
>> if that fails, use LookupAccountSid [currently it tries 
>> LookupAccountSid first, getting the sid from passwd]. 
>
>Actually it sounds good.  Do you have a patch?

Corinna,

Here it is. Actually, here they are. You can choose. In both cases
extract_nt_dom_user () first reads domain\username name in gecos.
If that fails it reads the sid (again in gecos !) and calls 
LookupAccountSid.

The "strict" version wants to see "U-domain\username" to avoid falling back.
The "soft" version keeps some of the existing features (that are never
exercised in normal cases) and accepts also "U-username" and the cygwin
user name. In those two cases LookupAccountSid is also called (to try to 
fill the domain, overwriting the username from passwd if it succeeds). 
Note that the cygwin user name is never parsed for domain\user.

So "strict" and "soft" can only differ if the U- field is messed up AND 
if LookupAccountSid fails, ALTHOUGH the sid is good in gecos!
The sid must be good for the seteuid to work.

My preference would be to use the strict version, the other one is 
offered in the spirit of making minimal changes. If you adopt it, delete
the third line below.

Pierre

2002-06-29  Pierre Humblet <pierre.humblet@ieee.org>

	security.cc (extract_nt_dom_user): Check for all buffer overflows.
	Call LookupAccountSid after trying to get domain & user from passwd. 
	Only accept correct syntax for U-domain\username. 
	(get_group_sidlist): Obtain the domain and user by calling 
	extract_nt_dom_user instead of LookupAccountSid.

-------------- next part --------------
--- security.cc.orig	2002-06-27 20:30:22.000000000 -0400
+++ security.cc	2002-06-29 16:36:10.000000000 -0400
@@ -60,43 +60,35 @@
 void
 extract_nt_dom_user (const struct passwd *pw, char *domain, char *user)
 {
-  cygsid psid;
-  DWORD ulen = UNLEN + 1;
-  DWORD dlen = INTERNET_MAX_HOST_NAME_LENGTH + 1;
-  SID_NAME_USE use;
-  char buf[INTERNET_MAX_HOST_NAME_LENGTH + UNLEN + 2];
-  char *c;
+  char *d, *u, *c;

-  strcpy (domain, "");
-  strcpy (buf, pw->pw_name);
   debug_printf ("pw_gecos = %x (%s)", pw->pw_gecos, pw->pw_gecos);

-  if (psid.getfrompw (pw) &&
-      LookupAccountSid (NULL, psid, user, &ulen, domain, &dlen, &use))
-    return;
-
-  if (pw->pw_gecos)
+  if ((d = strstr (pw->pw_gecos, "U-")) != NULL &&
+      (d == pw->pw_gecos || d[-1] == ','))
     {
-      if ((c = strstr (pw->pw_gecos, "U-")) != NULL &&
-	  (c == pw->pw_gecos || c[-1] == ','))
+      c = strchr (d + 2, ',');
+      if ((u = strchr (d + 2, '\\')) != NULL && (c == NULL || u < c) &&
+	  (u - d <= INTERNET_MAX_HOST_NAME_LENGTH + 2))
 	{
-	  buf[0] = '\0';
-	  strncat (buf, c + 2, INTERNET_MAX_HOST_NAME_LENGTH + UNLEN + 1);
-	  if ((c = strchr (buf, ',')) != NULL)
-	    *c = '\0';
+	  strlcpy(domain, d + 2, u - d - 1);
+	  if (c == NULL)
+	    c = u + UNLEN + 1;
+	  if (c - u <= UNLEN + 1)
+	    {
+	      strlcpy(user, u + 1, c - u);
+	      return;
+	    }
 	}
     }
-  if ((c = strchr (buf, '\\')) != NULL)
-    {
-      *c++ = '\0';
-      strcpy (domain, buf);
-      strcpy (user, c);
-    }
-  else
-    {
-      strcpy (domain, "");
-      strcpy (user, buf);
-    }
+
+  cygsid psid;
+  DWORD ulen = UNLEN + 1;
+  DWORD dlen = INTERNET_MAX_HOST_NAME_LENGTH + 1;
+  SID_NAME_USE use;
+  domain[0] = user[0] = 0;
+  if (psid.getfrompw (pw))
+    LookupAccountSid (NULL, psid, user, &ulen, domain, &dlen, &use);
 }

 extern "C" HANDLE
@@ -490,18 +482,9 @@
   char domain[INTERNET_MAX_HOST_NAME_LENGTH + 1];
   WCHAR wserver[INTERNET_MAX_HOST_NAME_LENGTH + 3];
   char server[INTERNET_MAX_HOST_NAME_LENGTH + 3];
-  DWORD ulen = sizeof (user);
-  DWORD dlen = sizeof (domain);
-  SID_NAME_USE use;
   cygsidlist sup_list;

   auth_pos = -1;
-  if (!LookupAccountSid (NULL, usersid, user, &ulen, domain, &dlen, &use))
-    {
-      debug_printf ("LookupAccountSid () %E");
-      __seterrno ();
-      return FALSE;
-    }

   grp_list += well_known_world_sid;
   if (usersid == well_known_system_sid)
@@ -511,6 +494,7 @@
     }
   else
     {
+      extract_nt_dom_user (pw, domain, user);
       if (!get_logon_server (domain, server, wserver))
 	return FALSE;
       if (my_grps)
-------------- next part --------------
--- security.cc.orig	2002-06-27 20:30:22.000000000 -0400
+++ security.cc	2002-06-29 16:34:10.000000000 -0400
@@ -60,43 +60,34 @@
 void
 extract_nt_dom_user (const struct passwd *pw, char *domain, char *user)
 {
-  cygsid psid;
-  DWORD ulen = UNLEN + 1;
-  DWORD dlen = INTERNET_MAX_HOST_NAME_LENGTH + 1;
-  SID_NAME_USE use;
-  char buf[INTERNET_MAX_HOST_NAME_LENGTH + UNLEN + 2];
-  char *c;
+  char *d, *u, *c;

-  strcpy (domain, "");
-  strcpy (buf, pw->pw_name);
+  domain[0] = 0;
+  strlcpy (user, pw->pw_name, UNLEN+1);
   debug_printf ("pw_gecos = %x (%s)", pw->pw_gecos, pw->pw_gecos);

-  if (psid.getfrompw (pw) &&
-      LookupAccountSid (NULL, psid, user, &ulen, domain, &dlen, &use))
-    return;
-
-  if (pw->pw_gecos)
-    {
-      if ((c = strstr (pw->pw_gecos, "U-")) != NULL &&
-	  (c == pw->pw_gecos || c[-1] == ','))
-	{
-	  buf[0] = '\0';
-	  strncat (buf, c + 2, INTERNET_MAX_HOST_NAME_LENGTH + UNLEN + 1);
-	  if ((c = strchr (buf, ',')) != NULL)
-	    *c = '\0';
-	}
-    }
-  if ((c = strchr (buf, '\\')) != NULL)
-    {
-      *c++ = '\0';
-      strcpy (domain, buf);
-      strcpy (user, c);
-    }
-  else
+  if ((d = strstr (pw->pw_gecos, "U-")) != NULL &&
+      (d == pw->pw_gecos || d[-1] == ','))
     {
-      strcpy (domain, "");
-      strcpy (user, buf);
-    }
+      c = strchr (d + 2, ',');
+      if ((u = strchr (d + 2, '\\')) == NULL || (c != NULL && u > c))
+	u = d + 1;
+      else if (u - d <= INTERNET_MAX_HOST_NAME_LENGTH + 2)
+	strlcpy(domain, d + 2, u - d - 1);
+      if (c == NULL)
+        c = u + UNLEN + 1;
+      if (c - u <= UNLEN + 1)
+	strlcpy(user, u + 1, c - u);
+    }
+  if (domain[0])
+    return;
+
+  cygsid psid;
+  DWORD ulen = UNLEN + 1;
+  DWORD dlen = INTERNET_MAX_HOST_NAME_LENGTH + 1;
+  SID_NAME_USE use;
+  if (psid.getfrompw (pw))
+    LookupAccountSid (NULL, psid, user, &ulen, domain, &dlen, &use);
 }

 extern "C" HANDLE
@@ -490,18 +481,9 @@
   char domain[INTERNET_MAX_HOST_NAME_LENGTH + 1];
   WCHAR wserver[INTERNET_MAX_HOST_NAME_LENGTH + 3];
   char server[INTERNET_MAX_HOST_NAME_LENGTH + 3];
-  DWORD ulen = sizeof (user);
-  DWORD dlen = sizeof (domain);
-  SID_NAME_USE use;
   cygsidlist sup_list;

   auth_pos = -1;
-  if (!LookupAccountSid (NULL, usersid, user, &ulen, domain, &dlen, &use))
-    {
-      debug_printf ("LookupAccountSid () %E");
-      __seterrno ();
-      return FALSE;
-    }

   grp_list += well_known_world_sid;
   if (usersid == well_known_system_sid)
@@ -511,6 +493,7 @@
     }
   else
     {
+      extract_nt_dom_user (pw, domain, user);
       if (!get_logon_server (domain, server, wserver))
 	return FALSE;
       if (my_grps)


More information about the Cygwin-patches mailing list