cr_canseeothergids(): Use real instead of effective group membership

Using the effective group and not the real one when testing membership
has the consequence that unprivileged processes cannot see setuid
commands they launch until these have relinquished their privileges.
This is also in contradiction with how the similar cr_canseeotheruids()
works, i.e., by taking into account real user IDs.

Fix this by substituting groupmember() with realgroupmember().  While
here, simplify the code.

PR:                     272093
Reviewed by:            mhorne
MFC after:              2 weeks
Sponsored by:           Kumacom SAS
Differential Revision:  https://reviews.freebsd.org/D40642
Differential Revision:  https://reviews.freebsd.org/D40644

(cherry picked from commit 91658080f1)
(cherry picked from commit 0452dd8413)
This commit is contained in:
Olivier Certner 2023-08-18 01:54:45 +02:00 committed by Mitchell Horne
parent d1fde7841f
commit f482bc9584
3 changed files with 15 additions and 18 deletions

View file

@ -97,7 +97,7 @@ and
are not members of any common group
.Po
as determined by
.Xr groupmember 9
.Xr realgroupmember 9
.Pc .
.It Bq Er ESRCH
Credentials

View file

@ -48,9 +48,9 @@ This function checks if a subject associated to credentials
is denied seeing a subject or object associated to credentials
.Fa u2
by a policy that requires both credentials to have at least one group in common.
For this determination, the effective and supplementary group IDs are used, but
not the real group IDs, as per
.Xr groupmember 9 .
For this determination, the real and supplementary group IDs are used, but
not the effective group IDs, as per
.Xr realgroupmember 9 .
.Pp
This policy is active if and only if the
.Xr sysctl 8
@ -79,5 +79,5 @@ Otherwise, it returns
.Er ESRCH .
.Sh SEE ALSO
.Xr cr_bsd_visible 9 ,
.Xr groupmember 9 ,
.Xr realgroupmember 9 ,
.Xr priv_check_cred 9

View file

@ -1404,21 +1404,18 @@ SYSCTL_INT(_security_bsd, OID_AUTO, see_other_gids, CTLFLAG_RW,
int
cr_canseeothergids(struct ucred *u1, struct ucred *u2)
{
int i, match;
if (!see_other_gids) {
match = 0;
for (i = 0; i < u1->cr_ngroups; i++) {
if (groupmember(u1->cr_groups[i], u2))
match = 1;
if (match)
break;
}
if (!match) {
if (priv_check_cred(u1, PRIV_SEEOTHERGIDS) != 0)
return (ESRCH);
}
if (realgroupmember(u1->cr_rgid, u2))
return (0);
for (int i = 1; i < u1->cr_ngroups; i++)
if (realgroupmember(u1->cr_groups[i], u2))
return (0);
if (priv_check_cred(u1, PRIV_SEEOTHERGIDS) != 0)
return (ESRCH);
}
return (0);
}