The branch main has been updated by kevans:
URL:
https://cgit.FreeBSD.org/src/commit/?id=1286fd8eccb78e540aae0b03c3a3f6d1865bdb81
commit 1286fd8eccb78e540aae0b03c3a3f6d1865bdb81
Author: Kyle Evans <kev...@freebsd.org>
AuthorDate: 2025-07-26 06:11:58 +0000
Commit: Kyle Evans <kev...@freebsd.org>
CommitDate: 2025-07-26 06:11:58 +0000
base: do a sweep of setgroups() that mean to clear the supplementaries
In the future, this will be beneficial as we move the egid out of the
groups list; there's no need to track the egid explicitly in our
supplemental groups, and doing so could become a security issue if
setgid() would not end up switching groups[0] as it does today and
we end up wanting to change our egid.
The rwhod diff is a little gratuitious, but I like patterns and
setgroups() -> setgid() -> setuid() is a lot more common than
setgid() -> setgroups() -> setuid().
This is sweep is not exhaustive and intentionally did not include
contrib/, crypto/ or tools/.
Reviewed by: olce
Differential Revision: https://reviews.freebsd.org/D51510
---
sbin/dhclient/dhclient.c | 2 +-
usr.sbin/rwhod/rwhod.c | 8 ++++----
usr.sbin/ypldap/ldapclient.c | 2 +-
usr.sbin/ypldap/ypldap.c | 2 +-
usr.sbin/ypldap/ypldap_dns.c | 2 +-
5 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/sbin/dhclient/dhclient.c b/sbin/dhclient/dhclient.c
index cbab3fa2973c..5d2a7453578b 100644
--- a/sbin/dhclient/dhclient.c
+++ b/sbin/dhclient/dhclient.c
@@ -539,7 +539,7 @@ main(int argc, char *argv[])
setproctitle("%s", ifi->name);
/* setgroups(2) is not permitted in capability mode. */
- if (setgroups(1, &pw->pw_gid) != 0)
+ if (setgroups(0, NULL) != 0)
error("can't restrict groups: %m");
if (caph_enter_casper() < 0)
diff --git a/usr.sbin/rwhod/rwhod.c b/usr.sbin/rwhod/rwhod.c
index 237663eef74d..b99e4ea74b5a 100644
--- a/usr.sbin/rwhod/rwhod.c
+++ b/usr.sbin/rwhod/rwhod.c
@@ -246,12 +246,12 @@ main(int argc, char *argv[])
syslog(LOG_ERR, "bind: %m");
exit(1);
}
- if (setgid(unpriv_gid) != 0) {
- syslog(LOG_ERR, "setgid: %m");
+ if (setgroups(0, NULL) != 0) {
+ syslog(LOG_ERR, "setgroups: %m");
exit(1);
}
- if (setgroups(1, &unpriv_gid) != 0) { /* XXX BOGUS groups[0] = egid */
- syslog(LOG_ERR, "setgroups: %m");
+ if (setgid(unpriv_gid) != 0) {
+ syslog(LOG_ERR, "setgid: %m");
exit(1);
}
if (setuid(unpriv_uid) != 0) {
diff --git a/usr.sbin/ypldap/ldapclient.c b/usr.sbin/ypldap/ldapclient.c
index acd4410d939f..a246a25a9605 100644
--- a/usr.sbin/ypldap/ldapclient.c
+++ b/usr.sbin/ypldap/ldapclient.c
@@ -385,7 +385,7 @@ ldapclient(int pipe_main2client[2])
ypldap_process = PROC_CLIENT;
#ifndef DEBUG
- if (setgroups(1, &pw->pw_gid) ||
+ if (setgroups(0, NULL) ||
setresgid(pw->pw_gid, pw->pw_gid, pw->pw_gid) ||
setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid))
fatal("cannot drop privileges");
diff --git a/usr.sbin/ypldap/ypldap.c b/usr.sbin/ypldap/ypldap.c
index 01b5955aa822..b9e938227831 100644
--- a/usr.sbin/ypldap/ypldap.c
+++ b/usr.sbin/ypldap/ypldap.c
@@ -602,7 +602,7 @@ main(int argc, char *argv[])
fatal("getpwnam");
#ifndef DEBUG
- if (setgroups(1, &pw->pw_gid) ||
+ if (setgroups(0, NULL) ||
setresgid(pw->pw_gid, pw->pw_gid, pw->pw_gid) ||
setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid))
fatal("cannot drop privileges");
diff --git a/usr.sbin/ypldap/ypldap_dns.c b/usr.sbin/ypldap/ypldap_dns.c
index 09ce636ebdc8..9dbbf26d237b 100644
--- a/usr.sbin/ypldap/ypldap_dns.c
+++ b/usr.sbin/ypldap/ypldap_dns.c
@@ -91,7 +91,7 @@ ypldap_dns(int pipe_ntp[2], struct passwd *pw)
setproctitle("dns engine");
close(pipe_ntp[0]);
- if (setgroups(1, &pw->pw_gid) ||
+ if (setgroups(0, NULL) ||
setresgid(pw->pw_gid, pw->pw_gid, pw->pw_gid) ||
setresuid(pw->pw_uid, pw->pw_uid, pw->pw_uid))
fatal("can't drop privileges");
