From 48fd05999b0f8e822fbf7069779378d103a35f5c Mon Sep 17 00:00:00 2001 From: Kyle Evans Date: Sat, 26 Jul 2025 01:11:58 -0500 Subject: [PATCH] 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 --- usr.sbin/chroot/chroot.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/usr.sbin/chroot/chroot.c b/usr.sbin/chroot/chroot.c index 32becaf1258..bd4932ee9b4 100644 --- a/usr.sbin/chroot/chroot.c +++ b/usr.sbin/chroot/chroot.c @@ -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;