Missing calls and unchecked return values of initgroups()
Jeffrey <[email protected]>
| Newsgroups | gmane.comp.gnu.inetutils.bugs |
|---|---|
| Message-ID | <CAJi1zq28jMUjrzUveUdD4wThXD+Nq81PzgTuaO+w+LSpMtFJYA@mail.gmail.com> |
Several initgroups() return values are not checked in inetutils programs. Others simply do not call initgroups() while relinquishing privileges. I found the following occurences of these issues: * inetd, uucpd, rshd, ftpd: missing return value check * tftpd: missing call This concern was raised by Alexander Peslyak on the oss-security mailing list: https://www.openwall.com/lists/oss-security/2023/12/30/2 This is indeed a security issue as these programs may not drop supplementary groups ownerships and a potential arbitrary code execution in subsequent code could lead to privilege escalation. POSIX have a rule related to this: https://wiki.sei.cmu.edu/confluence/display/c/POS36-C.+Observe+correct+revocation+order+while+relinquishing+privileges I am attaching a patch to both add the missing initgroups() return value checks and calls where needed for inetd, uucpd, rshd, ftpd and tftpd. Regards, -- Jeffrey BENCTEUX
0001-inetd-uucpd-rshd-ftpd-tftpd-fix-check-initgroups-ret.patch
(application/octet-stream, 3.8 KB)
From 8610717fb77d9aadc453a7651bc5288cf9f82036 Mon Sep 17 00:00:00 2001 From: Jeffrey Bencteux <[email protected]> Date: Sun, 7 Jan 2024 13:30:39 +0100 Subject: [PATCH] inetd,uucpd,rshd,ftpd,tftpd: fix check initgroups() return values Several initgroups() return values were not checked in inetd/uucpd/ rshd/ftpd code and tftpd was missing the call itself, potentially leading to security issues. Signed-off-by: Jeffrey Bencteux <[email protected]> --- NEWS | 6 ++++++ ftpd/ftpd.c | 6 +++++- src/inetd.c | 9 +++++++-- src/rshd.c | 6 +++++- src/tftpd.c | 21 +++++++++++++++++++++ src/uucpd.c | 6 +++++- 6 files changed, 49 insertions(+), 5 deletions(-) diff --git a/NEWS b/NEWS index 25ca5913..05da91db 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,12 @@ GNU inetutils NEWS -- history of user-visible changes. Thanks to Rui Chen and Caleb Xu, see <https://savannah.gnu.org/bugs/?65093>. +*** Avoid potential privilege escalations by checking and adding +initgroups() calls. +Thanks to Alexander Peslyak, see +<https://www.openwall.com/lists/oss-security/2023/12/30/2> +Reported by Jeffrey Bencteux + * Noteworthy changes in release 2.5 (2023-12-29) [stable] ** ftpd, rcp, rlogin, rsh, rshd, uucpd diff --git a/ftpd/ftpd.c b/ftpd/ftpd.c index 6df4f50a..3f9050f4 100644 --- a/ftpd/ftpd.c +++ b/ftpd/ftpd.c @@ -673,7 +673,11 @@ complete_login (struct credentials *pcred) } #ifdef HAVE_INITGROUPS - initgroups (pcred->name, pcred->gid); + if (initgroups (pcred->name, pcred->gid) == -1) + { + reply (550, "Can't change supplementary groups."); + goto bad; + } #endif /* open wtmp before chroot */ diff --git a/src/inetd.c b/src/inetd.c index 2d2bd52d..ae98f0c5 100644 --- a/src/inetd.c +++ b/src/inetd.c @@ -469,8 +469,13 @@ run_service (int ctrl, struct servtab *sep) _exit (EXIT_FAILURE); } #ifdef HAVE_INITGROUPS - initgroups (pwd->pw_name, - (grp && grp->gr_gid) ? grp->gr_gid : pwd->pw_gid); + if (initgroups (pwd->pw_name, + (grp && grp->gr_gid) ? grp->gr_gid : pwd->pw_gid) == -1) + { + syslog (LOG_ERR, "%s: can't initgroups for %s: %m", + sep->se_service, pwd->pw_name); + _exit (EXIT_FAILURE); + } #endif if (setuid (pwd->pw_uid) < 0) { diff --git a/src/rshd.c b/src/rshd.c index b5a8472a..e58cc613 100644 --- a/src/rshd.c +++ b/src/rshd.c @@ -1871,7 +1871,11 @@ doit (int sockfd, struct sockaddr *fromp, socklen_t fromlen) } #ifdef HAVE_INITGROUPS - initgroups (pwd->pw_name, pwd->pw_gid); /* BSD groups */ + if (initgroups (pwd->pw_name, pwd->pw_gid) == -1) /* BSD groups */ + { + rshd_error ("Cannot drop privileges (initgroups() failed)\n"); + exit (EXIT_FAILURE); + } #endif #ifdef WITH_PAM diff --git a/src/tftpd.c b/src/tftpd.c index e135ebcc..2dd35059 100644 --- a/src/tftpd.c +++ b/src/tftpd.c @@ -394,6 +394,27 @@ main (int argc, char *argv[]) } } +#ifdef HAVE_INITGROUPS + if (grp) + { + if (initgroups (pwd->pw_name, grp->gr_gid) == -1) + { + syslog (LOG_ERR, "initgroups: %m"); + nak (ENOUSER); + exit (EXIT_FAILURE); + } + } + else + { + if (initgroups (pwd->pw_name, pwd->pw_gid) == -1) + { + syslog (LOG_ERR, "initgroups: %m"); + nak (ENOUSER); + exit (EXIT_FAILURE); + } + } +#endif + if (setuid (pwd->pw_uid)) { syslog (LOG_ERR, "setuid: %m"); diff --git a/src/uucpd.c b/src/uucpd.c index aad85f22..d6653b96 100644 --- a/src/uucpd.c +++ b/src/uucpd.c @@ -257,7 +257,11 @@ doit (struct sockaddr *sap, socklen_t salen) return; } #ifdef HAVE_INITGROUPS - initgroups (pw->pw_name, pw->pw_gid); + if (initgroups (pw->pw_name, pw->pw_gid) == -1) + { + fprintf (stderr, "initgroups() failed"); + return; + } #endif if (chdir (pw->pw_dir) < 0) { -- 2.35.1