chroot: don't clobber the egid with the first supplemental group

There are two problems here, really:

1.) If -G is specified, the egid of the runner will get clobbered by
    the first supplemental group
2.) If both -G and -g are specified, the first supplemental group will
    get clobbered by the -g group

Ideally our users shouldn't have to understand the quirks of our
setgroups(2) and the manpage doesn't describe the group list as needing
to contain the egid, so populate the egid slot as necessary.

I note that this code seems to have already been marginally aware of the
historical behavior because it was allocating NGROUPS_MAX + 1, but this
is an artifact of a later conversion to doing dynamic allocations
instead of pushing NGROUPS_MAX arrays on the stack -- the original code
did in-fact only have an NGROUPS_MAX-sized array, and the layout was
still incorrect.

MFC after:	3 days
Reviewed by:	olce
Differential Revision:	https://reviews.freebsd.org/D51508
This commit is contained in:
Kyle Evans 2025-07-26 01:11:58 -05:00
parent 1f2c178e56
commit 48fd05999b

View file

@ -111,7 +111,12 @@ main(int argc, char *argv[])
ngroups_max = sysconf(_SC_NGROUPS_MAX) + 1;
if ((gidlist = malloc(sizeof(gid_t) * ngroups_max)) == NULL)
err(1, "malloc");
for (gids = 0;
/* Populate the egid slot in our groups to avoid accidents. */
if (gid == 0)
gidlist[0] = getegid();
else
gidlist[0] = gid;
for (gids = 1;
(p = strsep(&grouplist, ",")) != NULL && gids < ngroups_max; ) {
if (*p == '\0')
continue;