Skip to content

Commit

Permalink
Cygwin: uinfo: don't special case current user
Browse files Browse the repository at this point in the history
fetch_account_from_windows shortcuts the current user in that
it takes the user's domain SID and just adds the matching RID
from the token's primary group to create a group SID.

How wrong this is can be very simply reproduced:

Assuming you run a native process, like cmd, with primary group
set to the Administrators builtin group.  Run Cygwin's id(1) as
child process.  id(1) will print a non-existent group as primary
group and also add it to the group list.

This can only be avoided by not special casing the current user
and thus not creating a group SID from partial information.

Fixes: 6cc7c92 ("(pwdgrp::fetch_account_from_windows): Default primary group for the
current user to primary group from user token.")
Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
  • Loading branch information
github-cygwin committed Dec 2, 2022
1 parent dc7b673 commit a5bcfe6
Showing 1 changed file with 6 additions and 18 deletions.
24 changes: 6 additions & 18 deletions winsup/cygwin/uinfo.cc
Expand Up @@ -1855,7 +1855,6 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
gid_t gid = ILLEGAL_GID;
bool is_domain_account = true;
PCWSTR domain = NULL;
bool is_current_user = false;
char *shell = NULL;
char *home = NULL;
char *gecos = NULL;
Expand Down Expand Up @@ -2314,18 +2313,9 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
uid = posix_offset + sid_sub_auth_rid (sid);
if (!is_group () && acc_type == SidTypeUser)
{
/* Default primary group. If the sid is the current user, fetch
the default group from the current user token, otherwise make
the educated guess that the user is in group "Domain Users"
or "None". */
if (sid == cygheap->user.sid ())
{
is_current_user = true;
gid = posix_offset
+ sid_sub_auth_rid (cygheap->user.groups.pgsid);
}
else
gid = posix_offset + DOMAIN_GROUP_RID_USERS;
/* Default primary group. Make the educated guess that the user
is in group "Domain Users" or "None". */
gid = posix_offset + DOMAIN_GROUP_RID_USERS;
}

if (is_domain_account)
Expand All @@ -2336,11 +2326,9 @@ pwdgrp::fetch_account_from_windows (fetch_user_arg_t &arg, cyg_ldap *pldap)
/* On AD machines, use LDAP to fetch domain account infos. */
if (cygheap->dom.primary_dns_name ())
{
/* For the current user we got correctly cased username and
the primary group via process token. For any other user
we fetch it from AD and overwrite it. */
if (!is_current_user
&& cldap->fetch_ad_account (sid, false, domain))
/* Fetch primary group from AD and overwrite the one we
just guessed above. */
if (cldap->fetch_ad_account (sid, false, domain))
{
if ((val = cldap->get_account_name ()))
wcscpy (name, val);
Expand Down

0 comments on commit a5bcfe6

Please sign in to comment.