d/freebsd-nq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

In the C library, the setting up of the group array by various

Browse Source 
utilities is done by calling gr_addgid() for each group to be
added (usually found by traversing /etc/group) then calling the
setgroups() system call after the group set has been created.
The gr_addgid() function (helpfully?) deduplicates the addition
of group members. So, if you call it to add a group member that
already exists, it is just dropped. Because group[0] is the
effective group-ID and is over-written when a setgid program
is run, The value in group[0] is usually duplicated so that
group value is not lost when a setgid program is run.

Historically this happened because the group value indicated
in the password file also appears in /etc/group (e.g., if you
are group staff in the password file, you will also appear in
the staff line in /etc/group). But, with the addition of the
deduplication, the attempt to add group staff was lost because
it already appeared in group[0]. So, the fix is to deduplicate
starting from group[1] which allows a duplicate of the entry in
group[0], but not in later entries.

There is some confusion about the setgroups system call because in
BSD it has (always) set the entire group including the egid group
(in group[0]). However, in Linux, it skips over group[0] and starts
setting from group[1]. See this comment from linux_setgroups:

      /*
       * cr_groups[0] holds egid. Setting the whole set from
       * the supplied set will cause egid to be changed too.
       * Keep cr_groups[0] unchanged to prevent that.
       */

To make it clear what the BSD setgroups system call does, I
added the following paragraph to the setgroups(2) manual page:

   The first entry of the group array (gidset[0]) is used as the effective
   group-ID for the process.  This entry is over-written when a setgid
   program is run.  To avoid losing access to the privileges of the
   gidset[0] entry, it should be duplicated later in the group array.
   By convention, this happens because the group value indicated in the
   password file also appears in /etc/group.  The group value in the
   password file is placed in gidset[0] and that value then gets added a
   second time when the /etc/group file is scanned to create the group set.

Reported by: Paul McMath  paulm at tetrardus.net
Reviewed by: kib
MFC after:   2 weeks
This commit is contained in:
Kirk McKusick 2018-01-23 22:18:45 +00:00
parent 18d61b59bf
commit 8557409f20
2 changed files with 18 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
lib/libc/gen/getgrent.c
View File

@ -436,7 +436,7 @@ gr_addgid(gid_t gid, gid_t *groups, int maxgrp, int *grpcnt)
{
int ret, dupc;
for (dupc = 0; dupc < MIN(maxgrp, *grpcnt); dupc++) {
for (dupc = 1; dupc < MIN(maxgrp, *grpcnt); dupc++) {
if (groups[dupc] == gid)
return 1;
}

17
lib/libc/sys/setgroups.2
View File

@ -56,6 +56,23 @@ more than
.Dv {NGROUPS_MAX}+1 .
.Pp
Only the super-user may set a new group list.
.Pp
The first entry of the group array
.Pq Va gidset[0]
is used as the effective group-ID for the process.
This entry is over-written when a setgid program is run.
To avoid losing access to the privileges of the
.Va gidset[0]
entry, it should be duplicated later in the group array.
By convention,
this happens because the group value indicated
in the password file also appears in
.Pa /etc/group .
The group value in the password file is placed in
.Va gidset[0]
and that value then gets added a second time when the
.Pa /etc/group
file is scanned to create the group set.
.Sh RETURN VALUES
.Rv -std setgroups
.Sh ERRORS