Page MenuHomeFreeBSD

D51648.id159740.diff
No OneTemporary

D51648.id159740.diff

diff --git a/lib/libsys/getgroups.2 b/lib/libsys/getgroups.2
--- a/lib/libsys/getgroups.2
+++ b/lib/libsys/getgroups.2
@@ -25,7 +25,7 @@
.\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
.\" SUCH DAMAGE.
.\"
-.Dd January 21, 2011
+.Dd August 1, 2025
.Dt GETGROUPS 2
.Os
.Sh NAME
@@ -41,8 +41,8 @@
The
.Fn getgroups
system call
-gets the current group access list of the user process
-and stores it in the array
+gets the current supplementary groups of the user process and stores it in the
+array
.Fa gidset .
The
.Fa gidsetlen
@@ -54,7 +54,7 @@
system call
returns the actual number of groups returned in
.Fa gidset .
-At least one and as many as {NGROUPS_MAX}+1 values may be returned.
+As many as {NGROUPS_MAX} values may be returned.
If
.Fa gidsetlen
is zero,
@@ -102,3 +102,10 @@
.Fn getgroups
system call appeared in
.Bx 4.2 .
+.Pp
+Before
+.Fx 15.0 ,
+the
+.Fn getgroups
+system call always returned the effective group ID for the process as the first
+element of the array, before the supplementary groups.
diff --git a/lib/libsys/setgroups.2 b/lib/libsys/setgroups.2
--- a/lib/libsys/setgroups.2
+++ b/lib/libsys/setgroups.2
@@ -25,7 +25,7 @@
.\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
.\" SUCH DAMAGE.
.\"
-.Dd January 19, 2018
+.Dd August 1, 2025
.Dt SETGROUPS 2
.Os
.Sh NAME
@@ -42,7 +42,7 @@
The
.Fn setgroups
system call
-sets the group access list of the current user process
+sets the supplementary group list of the current user process
according to the array
.Fa gidset .
The
@@ -50,26 +50,12 @@
argument
indicates the number of entries in the array and must be no
more than
-.Dv {NGROUPS_MAX}+1 .
-.Pp
-Only the super-user may set a new group list.
+.Dv {NGROUPS_MAX} .
+The
+.Fa ngroups
+argument may be set to 0 to clear the supplementary 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.
+Only the super-user may set a new supplementary group list.
.Sh RETURN VALUES
.Rv -std setgroups
.Sh ERRORS
@@ -99,3 +85,11 @@
.Fn setgroups
system call appeared in
.Bx 4.2 .
+.Pp
+Before
+.Fx 15.0 ,
+the
+.Fn setgroups
+system call would set the effective group ID for the process to the first
+element of
+.Fa gidset .
diff --git a/sbin/hastd/subr.c b/sbin/hastd/subr.c
--- a/sbin/hastd/subr.c
+++ b/sbin/hastd/subr.c
@@ -207,10 +207,8 @@
}
}
PJDLOG_VERIFY(chdir("/") == 0);
- gidset[0] = pw->pw_gid;
- if (setgroups(1, gidset) == -1) {
- pjdlog_errno(LOG_ERR, "Unable to set groups to gid %u",
- (unsigned int)pw->pw_gid);
+ if (setgroups(0, NULL) == -1) {
+ pjdlog_errno(LOG_ERR, "Unable to drop supplementary groups");
return (-1);
}
if (setgid(pw->pw_gid) == -1) {
@@ -287,8 +285,7 @@
PJDLOG_VERIFY(egid == pw->pw_gid);
PJDLOG_VERIFY(sgid == pw->pw_gid);
PJDLOG_VERIFY(getgroups(0, NULL) == 1);
- PJDLOG_VERIFY(getgroups(1, gidset) == 1);
- PJDLOG_VERIFY(gidset[0] == pw->pw_gid);
+ PJDLOG_VERIFY(getgroups(1, gidset) == 0);
pjdlog_debug(1,
"Privileges successfully dropped using %s%s+setgid+setuid.",
diff --git a/sys/kern/kern_prot.c b/sys/kern/kern_prot.c
--- a/sys/kern/kern_prot.c
+++ b/sys/kern/kern_prot.c
@@ -320,18 +320,20 @@
sys_getgroups(struct thread *td, struct getgroups_args *uap)
{
struct ucred *cred;
- gid_t *ugidset;
int ngrp, error;
+#ifdef COMPAT_FREEBSD14
+ int osrel = td->td_proc->p_osrel;
+ bool getgroups_compat;
+#endif
cred = td->td_ucred;
- /*
- * cr_gid has been moved out of cr_groups, but we'll continue exporting
- * the egid as groups[0] for the time being until we audit userland for
- * any surprises.
- */
- ngrp = cred->cr_ngroups + 1;
-
+ ngrp = cred->cr_ngroups;
+#ifdef COMPAT_FREEBSD14
+ getgroups_compat = osrel < P_OSREL_SETGROUPS_EGID;
+ if (getgroups_compat)
+ ngrp++;
+#endif
if (uap->gidsetsize == 0) {
error = 0;
goto out;
@@ -339,14 +341,15 @@
if (uap->gidsetsize < ngrp)
return (EINVAL);
- ugidset = uap->gidset;
- error = copyout(&cred->cr_gid, ugidset, sizeof(*ugidset));
- if (error != 0)
- goto out;
-
- if (ngrp > 1)
- error = copyout(cred->cr_groups, ugidset + 1,
- (ngrp - 1) * sizeof(*ugidset));
+#ifdef COMPAT_FREEBSD14
+ if (getgroups_compat) {
+ error = copyout(&cred->cr_gid, uap->gidset, sizeof(gid_t));
+ if (error != 0)
+ error = copyout(cred->cr_groups, uap->gidset + 1,
+ (ngrp - 1) * sizeof(gid_t));
+ } else
+#endif
+ error = copyout(cred->cr_groups, uap->gidset, ngrp * sizeof(gid_t));
out:
td->td_retval[0] = ngrp;
return (error);
@@ -1198,7 +1201,18 @@
{
gid_t smallgroups[CRED_SMALLGROUPS_NB];
gid_t *groups;
- int gidsetsize, error;
+ int gidsetsize, error, maxsize;
+#ifdef COMPAT_FREEBSD14
+ int osrel = td->td_proc->p_osrel;
+ bool setgroups_compat;
+#endif
+
+ maxsize = ngroups_max;
+#ifdef COMPAT_FREEBSD14
+ setgroups_compat = osrel < P_OSREL_SETGROUPS_EGID;
+ if (setgroups_compat)
+ maxsize++; /* One more to account for the egid */
+#endif
/*
* Sanity check size now to avoid passing too big a value to copyin(),
@@ -1210,8 +1224,7 @@
* setgroups() differ.
*/
gidsetsize = uap->gidsetsize;
- /* XXXKE Limit to ngroups_max when we change the userland interface. */
- if (gidsetsize > ngroups_max + 1 || gidsetsize < 0)
+ if (gidsetsize > maxsize || gidsetsize < 0)
return (EINVAL);
if (gidsetsize > CRED_SMALLGROUPS_NB)
@@ -1220,8 +1233,19 @@
groups = smallgroups;
error = copyin(uap->gidset, groups, gidsetsize * sizeof(gid_t));
- if (error == 0)
+ if (error == 0) {
+#ifdef COMPAT_FREEBSD14
+ if (gidsetsize > 0 && setgroups_compat) {
+ gidsetsize--;
+ error = kern_setgroups(td, &gidsetsize, groups + 1);
+ } else
+#endif
error = kern_setgroups(td, &gidsetsize, groups);
+#ifdef COMPAT_FREEBSD14
+ if (uap->gidsetsize > 0 && error == 0 && setgroups_compat)
+ td->td_proc->p_ucred->cr_gid = groups[0];
+#endif
+ }
if (groups != smallgroups)
free(groups, M_TEMP);
@@ -1238,35 +1262,17 @@
struct proc *p = td->td_proc;
struct ucred *newcred, *oldcred;
int ngrp, error;
- gid_t egid;
ngrp = *ngrpp;
/* Sanity check size. */
- /* XXXKE Limit to ngroups_max when we change the userland interface. */
- if (ngrp < 0 || ngrp > ngroups_max + 1)
+ if (ngrp < 0 || ngrp > ngroups_max)
return (EINVAL);
AUDIT_ARG_GROUPSET(groups, ngrp);
- /*
- * setgroups(0, NULL) is a legitimate way of clearing the groups vector
- * on non-BSD systems (which generally do not have the egid in the
- * groups[0]). We risk security holes when running non-BSD software if
- * we do not do the same. So we allow and treat 0 for 'ngrp' specially
- * below (twice).
- */
- if (ngrp != 0) {
- /*
- * To maintain userland compat for now, we use the first group
- * as our egid and we'll use the rest as our supplemental
- * groups.
- */
- egid = groups[0];
- ngrp--;
- groups++;
- groups_normalize(&ngrp, groups);
- *ngrpp = ngrp;
- }
+ groups_normalize(&ngrp, groups);
+ *ngrpp = ngrp;
+
newcred = crget();
crextend(newcred, ngrp);
PROC_LOCK(p);
@@ -1289,15 +1295,7 @@
if (error)
goto fail;
- /*
- * If some groups were passed, the first one is currently the desired
- * egid. This code is to be removed (along with some commented block
- * above) when setgroups() is changed to take only supplementary groups.
- */
- if (ngrp != 0)
- newcred->cr_gid = egid;
crsetgroups_internal(newcred, ngrp, groups);
-
setsugid(p);
proc_set_cred(p, newcred);
PROC_UNLOCK(p);
diff --git a/sys/sys/param.h b/sys/sys/param.h
--- a/sys/sys/param.h
+++ b/sys/sys/param.h
@@ -109,6 +109,7 @@
#define P_OSREL_ARM64_SPSR 1400084
#define P_OSREL_TLSBASE 1500044
#define P_OSREL_EXTERRCTL 1500045
+#define P_OSREL_SETGROUPS_EGID 1500056
#define P_OSREL_MAJOR(x) ((x) / 100000)
#endif
diff --git a/usr.bin/newgrp/newgrp.c b/usr.bin/newgrp/newgrp.c
--- a/usr.bin/newgrp/newgrp.c
+++ b/usr.bin/newgrp/newgrp.c
@@ -186,7 +186,7 @@
}
}
- ngrps_max = sysconf(_SC_NGROUPS_MAX) + 1;
+ ngrps_max = sysconf(_SC_NGROUPS_MAX);
if ((grps = malloc(sizeof(gid_t) * ngrps_max)) == NULL)
err(1, "malloc");
if ((ngrps = getgroups(ngrps_max, (gid_t *)grps)) < 0) {
@@ -194,7 +194,12 @@
goto end;
}
- /* Remove requested gid from supp. list if it exists. */
+ /*
+ * Remove requested gid from supp. list if it exists and doesn't match
+ * our prior egid -- this exception is to avoid providing the user a
+ * means to get rid of a group that could be used for, e.g., negative
+ * permissions.
+ */
if (grp->gr_gid != egid && inarray(grp->gr_gid, grps, ngrps)) {
for (i = 0; i < ngrps; i++)
if (grps[i] == grp->gr_gid)
@@ -217,10 +222,9 @@
goto end;
}
PRIV_END;
- grps[0] = grp->gr_gid;
/* Add old effective gid to supp. list if it does not exist. */
- if (egid != grp->gr_gid && !inarray(egid, grps, ngrps)) {
+ if (!inarray(egid, grps, ngrps)) {
if (ngrps == ngrps_max)
warnx("too many groups");
else {
diff --git a/usr.bin/quota/quota.c b/usr.bin/quota/quota.c
--- a/usr.bin/quota/quota.c
+++ b/usr.bin/quota/quota.c
@@ -100,8 +100,7 @@
int
main(int argc, char *argv[])
{
- int ngroups;
- gid_t mygid, gidset[NGROUPS];
+ int ngroups;
int i, ch, gflag = 0, uflag = 0, errflag = 0;
while ((ch = getopt(argc, argv, "f:ghlrquv")) != -1) {
@@ -142,11 +141,15 @@
if (uflag)
errflag += showuid(getuid());
if (gflag) {
+ gid_t mygid, myegid, gidset[NGROUPS_MAX];
+
mygid = getgid();
- ngroups = getgroups(NGROUPS, gidset);
+ errflag += showgid(mygid);
+ myegid = getegid();
+ errflag += showgid(myegid);
+ ngroups = getgroups(NGROUPS_MAX, gidset);
if (ngroups < 0)
err(1, "getgroups");
- errflag += showgid(mygid);
for (i = 0; i < ngroups; i++)
if (gidset[i] != mygid)
errflag += showgid(gidset[i]);
diff --git a/usr.sbin/chroot/chroot.c b/usr.sbin/chroot/chroot.c
--- a/usr.sbin/chroot/chroot.c
+++ b/usr.sbin/chroot/chroot.c
@@ -139,15 +139,10 @@
if (group != NULL)
gid = resolve_group(group);
- ngroups_max = sysconf(_SC_NGROUPS_MAX) + 1;
+ ngroups_max = sysconf(_SC_NGROUPS_MAX);
if ((gidlist = malloc(sizeof(gid_t) * ngroups_max)) == NULL)
err(1, "malloc");
- /* Populate the egid slot in our groups to avoid accidents. */
- if (gid == 0)
- gidlist[0] = getegid();
- else
- gidlist[0] = gid;
- for (gids = 1;
+ for (gids = 0;
(p = strsep(&grouplist, ",")) != NULL && gids < ngroups_max; ) {
if (*p == '\0')
continue;
diff --git a/usr.sbin/lpr/lpc/lpc.c b/usr.sbin/lpr/lpc/lpc.c
--- a/usr.sbin/lpr/lpc/lpc.c
+++ b/usr.sbin/lpr/lpc/lpc.c
@@ -358,6 +358,8 @@
err(1, "getgroups");
}
gid = gptr->gr_gid;
+ if (gid == getegid())
+ return(1);
for (i = 0; i < ngroups; i++)
if (gid == groups[i])
return(1);

File Metadata

Mime Type
text/plain
Expires
Sat, Aug 29, 3:50 AM (17 h, 58 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
37495120
Default Alt Text
D51648.id159740.diff (11 KB)

Event Timeline