I found more occurences of unchecked values for set*id() functions in other
inetutils programs: ftpd, rcp.
It has different security impact if it can be triggered:
* rcp: local privilege escalation to the user running the binary
* ftpd: undefined behaviour without privilege escalation as all calls are
to seteuid(0) (gaining root privileges, not dropping it)
I am attaching a consolidated patch to fix these and the previous ones.
---
>From 05bca16ab557abe18c9deca0e64e2ce5a43cb875 Mon Sep 17 00:00:00 2001
From: Jeffrey Bencteux <jeffbenct...@gmail.com>
Date: Fri, 30 Jun 2023 19:02:45 +0200
Subject: [PATCH] ftpd,rcp,rlogin,rsh,rshd,uucpd: fix: check set*id() return
values
Several setuid(), setgid(), seteuid() and setguid() return values
were not checked in ftpd/rcp/rlogin/rsh/rshd/uucpd code potentially
leading to potential security issues.
---
ftpd/ftpd.c | 10 +++++++---
src/rcp.c | 39 +++++++++++++++++++++++++++++++++------
src/rlogin.c | 11 +++++++++--
src/rsh.c | 25 +++++++++++++++++++++----
src/rshd.c | 20 +++++++++++++++++---
src/uucpd.c | 15 +++++++++++++--
6 files changed, 100 insertions(+), 20 deletions(-)
diff --git a/ftpd/ftpd.c b/ftpd/ftpd.c
index 92b2cca..28dd523 100644
--- a/ftpd/ftpd.c
+++ b/ftpd/ftpd.c
@@ -862,7 +862,9 @@ end_login (struct credentials *pcred)
char *remotehost = pcred->remotehost;
int atype = pcred->auth_type;
- seteuid ((uid_t) 0);
+ if (seteuid ((uid_t) 0) == -1)
+ _exit (EXIT_FAILURE);
+
if (pcred->logged_in)
{
logwtmp_keep_open (ttyline, "", "");
@@ -1151,7 +1153,8 @@ getdatasock (const char *mode)
if (data >= 0)
return fdopen (data, mode);
- seteuid ((uid_t) 0);
+ if (seteuid ((uid_t) 0) == -1)
+ _exit (EXIT_FAILURE);
s = socket (ctrl_addr.ss_family, SOCK_STREAM, 0);
if (s < 0)
goto bad;
@@ -1978,7 +1981,8 @@ passive (int epsv, int af)
else /* !AF_INET6 */
((struct sockaddr_in *) &pasv_addr)->sin_port = 0;
- seteuid ((uid_t) 0);
+ if (seteuid ((uid_t) 0) == -1)
+ _exit (EXIT_FAILURE);
if (bind (pdata, (struct sockaddr *) &pasv_addr, pasv_addrlen) < 0)
{
if (seteuid ((uid_t) cred.uid))
diff --git a/src/rcp.c b/src/rcp.c
index 75adb25..cdcf850 100644
--- a/src/rcp.c
+++ b/src/rcp.c
@@ -345,14 +345,23 @@ main (int argc, char *argv[])
if (from_option)
{ /* Follow "protocol", send data. */
response ();
- setuid (userid);
+
+ if (setuid (userid) == -1)
+ {
+ error (EXIT_FAILURE, 0, "Could not drop privileges (setuid()
failed)");
+ }
+
source (argc, argv);
exit (errs);
}
if (to_option)
{ /* Receive data. */
- setuid (userid);
+ if (setuid (userid) == -1)
+ {
+ error (EXIT_FAILURE, 0, "Could not drop privileges (setuid()
failed)");
+ }
+
sink (argc, argv);
exit (errs);
}
@@ -537,7 +546,11 @@ toremote (char *targ, int argc, char *argv[])
if (response () < 0)
exit (EXIT_FAILURE);
free (bp);
- setuid (userid);
+
+ if (setuid (userid) == -1)
+ {
+ error (EXIT_FAILURE, 0, "Could not drop privileges
(setuid() failed)");
+ }
}
source (1, argv + i);
close (rem);
@@ -630,7 +643,12 @@ tolocal (int argc, char *argv[])
++errs;
continue;
}
- seteuid (userid);
+
+ if (seteuid (userid) == -1)
+ {
+ error (EXIT_FAILURE, 0, "Could not drop privileges (seteuid()
failed)");
+ }
+
#if defined IP_TOS && defined IPPROTO_IP && defined IPTOS_THROUGHPUT
sslen = sizeof (ss);
(void) getpeername (rem, (struct sockaddr *) &ss, &sslen);
@@ -643,7 +661,12 @@ tolocal (int argc, char *argv[])
#endif
vect[0] = target;
sink (1, vect);
- seteuid (effuid);
+
+ if (seteuid (effuid) == -1)
+ {
+ error (EXIT_FAILURE, 0, "Could not drop privileges (seteuid()
failed)");
+ }
+
close (rem);
rem = -1;
#ifdef SHISHI
@@ -1441,7 +1464,11 @@ susystem (char *s, int userid)
return (127);
case 0:
- setuid (userid);
+ if (setuid (userid) == -1)
+ {
+ error (EXIT_FAILURE, 0, "Could not drop privileges (setuid()
failed)");
+ }
+
execl (PATH_BSHELL, "sh", "-c", s, NULL);
_exit (127);
}
diff --git a/src/rlogin.c b/src/rlogin.c
index aa6426f..c543de0 100644
--- a/src/rlogin.c
+++ b/src/rlogin.c
@@ -647,8 +647,15 @@ try_connect:
/* Now change to the real user ID. We have to be set-user-ID root
to get the privileged port that rcmd () uses. We now want, however,
to run as the real user who invoked us. */
- seteuid (uid);
- setuid (uid);
+ if (seteuid (uid) == -1)
+ {
+ error (EXIT_FAILURE, 0, "Could not drop privileges (seteuid()
failed)");
+ }
+
+ if (setuid (uid) == -1)
+ {
+ error (EXIT_FAILURE, 0, "Could not drop privileges (setuid() failed)");
+ }
doit (&osmask); /* The old mask will activate SIGURG and SIGUSR1! */
diff --git a/src/rsh.c b/src/rsh.c
index 2d622ca..6f60667 100644
--- a/src/rsh.c
+++ b/src/rsh.c
@@ -276,8 +276,17 @@ main (int argc, char **argv)
{
if (asrsh)
*argv = (char *) "rlogin";
- seteuid (getuid ());
- setuid (getuid ());
+
+ if (seteuid (getuid ()) == -1)
+ {
+ error (EXIT_FAILURE, errno, "seteuid() failed");
+ }
+
+ if (setuid (getuid ()) == -1)
+ {
+ error (EXIT_FAILURE, errno, "setuid() failed");
+ }
+
execv (PATH_RLOGIN, argv);
error (EXIT_FAILURE, errno, "cannot execute %s", PATH_RLOGIN);
}
@@ -541,8 +550,16 @@ try_connect:
error (0, errno, "setsockopt DEBUG (ignored)");
}
- seteuid (uid);
- setuid (uid);
+ if (seteuid (uid) == -1)
+ {
+ error (EXIT_FAILURE, errno, "seteuid() failed");
+ }
+
+ if (setuid (uid) == -1)
+ {
+ error (EXIT_FAILURE, errno, "setuid() failed");
+ }
+
#ifdef HAVE_SIGACTION
sigemptyset (&sigs);
sigaddset (&sigs, SIGINT);
diff --git a/src/rshd.c b/src/rshd.c
index d1c0d0c..707790e 100644
--- a/src/rshd.c
+++ b/src/rshd.c
@@ -1847,8 +1847,18 @@ doit (int sockfd, struct sockaddr *fromp, socklen_t
fromlen)
pwd->pw_shell = PATH_BSHELL;
/* Set the gid, then uid to become the user specified by "locuser" */
- setegid ((gid_t) pwd->pw_gid);
- setgid ((gid_t) pwd->pw_gid);
+ if (setegid ((gid_t) pwd->pw_gid) == -1)
+ {
+ rshd_error ("Cannot drop privileges (setegid() failed)\n");
+ exit (EXIT_FAILURE);
+ }
+
+ if (setgid ((gid_t) pwd->pw_gid) == -1)
+ {
+ rshd_error ("Cannot drop privileges (setgid() failed)\n");
+ exit (EXIT_FAILURE);
+ }
+
#ifdef HAVE_INITGROUPS
initgroups (pwd->pw_name, pwd->pw_gid); /* BSD groups */
#endif
@@ -1870,7 +1880,11 @@ doit (int sockfd, struct sockaddr *fromp, socklen_t
fromlen)
}
#endif /* WITH_PAM */
- setuid ((uid_t) pwd->pw_uid);
+ if (setuid ((uid_t) pwd->pw_uid) == -1)
+ {
+ rshd_error ("Cannot drop privileges (setuid() failed)\n");
+ exit (EXIT_FAILURE);
+ }
/* We'll execute the client's command in the home directory
* of locuser. Note, that the chdir must be executed after
diff --git a/src/uucpd.c b/src/uucpd.c
index 107589e..29cfce3 100644
--- a/src/uucpd.c
+++ b/src/uucpd.c
@@ -252,7 +252,12 @@ doit (struct sockaddr *sap, socklen_t salen)
snprintf (Username, sizeof (Username), "USER=%s", user);
snprintf (Logname, sizeof (Logname), "LOGNAME=%s", user);
dologin (pw, sap, salen);
- setgid (pw->pw_gid);
+
+ if (setgid (pw->pw_gid) == -1)
+ {
+ fprintf (stderr, "setgid() failed");
+ return;
+ }
#ifdef HAVE_INITGROUPS
initgroups (pw->pw_name, pw->pw_gid);
#endif
@@ -261,7 +266,13 @@ doit (struct sockaddr *sap, socklen_t salen)
fprintf (stderr, "Login incorrect.");
return;
}
- setuid (pw->pw_uid);
+
+ if (setuid (pw->pw_uid) == -1)
+ {
+ fprintf (stderr, "setuid() failed");
+ return;
+ }
+
execl (uucico_location, "uucico", NULL);
perror ("uucico server: execl");
}
--
2.35.1
--
Jeffrey BENCTEUX
Le sam. 1 juil. 2023 à 10:30, Jeffrey <jeffbenct...@gmail.com> a écrit :
> Hi,
>
> Several setuid(), setgid(), seteuid() and setguid() return values are not
> checked in rlogin/rsh/rshd/uucpd code:
>
> rlogin.c:
>
> 647 /* Now change to the real user ID. We have to be set-user-ID
> root
> 648 to get the privileged port that rcmd () uses. We now want,
> however,
> 649 to run as the real user who invoked us. */
> 650 seteuid (uid);
> 651 setuid (uid);
> 652
> 653 doit (&osmask); /* The old mask will activate SIGURG and
> SIGUSR1! */
>
> rsh.c:
>
> 274 /* If no further arguments, must have been called as rlogin. */
> 275 if (!argv[index])
> 276 {
> 277 if (asrsh)
> 278 *argv = (char *) "rlogin";
> 279 seteuid (getuid ());
> 280 setuid (getuid ());
> 281 execv (PATH_RLOGIN, argv);
> 282 error (EXIT_FAILURE, errno, "cannot execute %s",
> PATH_RLOGIN);
> 283 }
>
> [...]
>
> 541 error (0, errno, "setsockopt DEBUG (ignored)");
> 542 }
> 543
> 544 seteuid (uid);
> 545 setuid (uid);
> 546 #ifdef HAVE_SIGACTION
> 547 sigemptyset (&sigs);
>
> rshd.c:
>
> 1846 if (*pwd->pw_shell == '\0')
> 1847 pwd->pw_shell = PATH_BSHELL;
> 1848
> 1849 /* Set the gid, then uid to become the user specified by
> "locuser" */
> 1850 setegid ((gid_t) pwd->pw_gid);
> 1851 setgid ((gid_t) pwd->pw_gid);
> 1852 #ifdef HAVE_INITGROUPS
> 1853 initgroups (pwd->pw_name, pwd->pw_gid); /* BSD groups */
>
> [...]
>
> 1871 #endif /* WITH_PAM */
> 1872
> 1873 setuid ((uid_t) pwd->pw_uid);
> 1874
> 1875 /* We'll execute the client's command in the home directory
> 1876 * of locuser. Note, that the chdir must be executed after
> 1877 * setuid(), otherwise it may fail on NFS mounted directories
> 1878 * (root mapped to nobody).
> 1879 */
> 1880 if (chdir (pwd->pw_dir) < 0)
>
> uucpd.c:
>
> 252 snprintf (Username, sizeof (Username), "USER=%s", user);
> 253 snprintf (Logname, sizeof (Logname), "LOGNAME=%s", user);
> 254 dologin (pw, sap, salen);
> 255 setgid (pw->pw_gid); <=========
> 256 #ifdef HAVE_INITGROUPS
> 257 initgroups (pw->pw_name, pw->pw_gid);
> 258 #endif
> 259 if (chdir (pw->pw_dir) < 0)
> 260 {
> 261 fprintf (stderr, "Login incorrect.");
> 262 return;
> 263 }
> 264 setuid (pw->pw_uid); <=========
> 265 execl (uucico_location, "uucico", NULL);
> 266 perror ("uucico server: execl");
>
>
> There are cases where set*id() functions can fail, for example multiple
> calls to the clone() function can cause setuid() to fail when the user
> process limit is reached.
>
> man 2 setuid():
>
> RETURN VALUE
> On success, zero is returned. On error, -1 is returned, and errno
> is set to indicate the error.
>
> Note: there are cases where setuid() can fail even when the caller
> is UID 0; it is a grave security error to omit checking for a failure
> return from setuid().
>
> The above code could be abused in different ways to trigger such failures,
> potentially remotely in the case of rshd and uucpd.
>
> Here are some example scenarios:
>
> * rshd: if daemon run as root, privilege escalation is possible as any
> user logging in after a set*id() failure would have its session started as
> root.
> * rlogin: potential local privilege escalation as the binary is setUID root
> * uucpd: potential remote privilege escalation to root for already valid
> users
>
> I believe the below patch mitigates the issue, let me know if that suits
> you.
>
> Regards,
>
> ---
>
> From: Jeffrey Bencteux <jeffbenct...@gmail.com>
> Date: Fri, 30 Jun 2023 19:02:45 +0200
> Subject: [PATCH] rlogin,rsh,rshd,uucpd: fix: check set*id() return values
>
> Several setuid(), setgid(), seteuid() and setguid() return values
> were not checked in rlogin/rsh/rshd/uucpd code potentially
> leading to security issues.
> ---
> src/rlogin.c | 11 +++++++++--
> src/rsh.c | 25 +++++++++++++++++++++----
> src/rshd.c | 20 +++++++++++++++++---
> src/uucpd.c | 15 +++++++++++++--
> 4 files changed, 60 insertions(+), 11 deletions(-)
>
> diff --git a/src/rlogin.c b/src/rlogin.c
> index aa6426f..c543de0 100644
> --- a/src/rlogin.c
> +++ b/src/rlogin.c
> @@ -647,8 +647,15 @@ try_connect:
> /* Now change to the real user ID. We have to be set-user-ID root
> to get the privileged port that rcmd () uses. We now want, however,
> to run as the real user who invoked us. */
> - seteuid (uid);
> - setuid (uid);
> + if (seteuid (uid) == -1)
> + {
> + error (EXIT_FAILURE, 0, "Could not drop privileges (seteuid()
> failed)");
> + }
> +
> + if (setuid (uid) == -1)
> + {
> + error (EXIT_FAILURE, 0, "Could not drop privileges (setuid()
> failed)");
> + }
>
> doit (&osmask); /* The old mask will activate SIGURG and SIGUSR1! */
>
> diff --git a/src/rsh.c b/src/rsh.c
> index 2d622ca..6f60667 100644
> --- a/src/rsh.c
> +++ b/src/rsh.c
> @@ -276,8 +276,17 @@ main (int argc, char **argv)
> {
> if (asrsh)
> *argv = (char *) "rlogin";
> - seteuid (getuid ());
> - setuid (getuid ());
> +
> + if (seteuid (getuid ()) == -1)
> + {
> + error (EXIT_FAILURE, errno, "seteuid() failed");
> + }
> +
> + if (setuid (getuid ()) == -1)
> + {
> + error (EXIT_FAILURE, errno, "setuid() failed");
> + }
> +
> execv (PATH_RLOGIN, argv);
> error (EXIT_FAILURE, errno, "cannot execute %s", PATH_RLOGIN);
> }
> @@ -541,8 +550,16 @@ try_connect:
> error (0, errno, "setsockopt DEBUG (ignored)");
> }
>
> - seteuid (uid);
> - setuid (uid);
> + if (seteuid (uid) == -1)
> + {
> + error (EXIT_FAILURE, errno, "seteuid() failed");
> + }
> +
> + if (setuid (uid) == -1)
> + {
> + error (EXIT_FAILURE, errno, "setuid() failed");
> + }
> +
> #ifdef HAVE_SIGACTION
> sigemptyset (&sigs);
> sigaddset (&sigs, SIGINT);
> diff --git a/src/rshd.c b/src/rshd.c
> index d1c0d0c..707790e 100644
> --- a/src/rshd.c
> +++ b/src/rshd.c
> @@ -1847,8 +1847,18 @@ doit (int sockfd, struct sockaddr *fromp, socklen_t
> fromlen)
> pwd->pw_shell = PATH_BSHELL;
>
> /* Set the gid, then uid to become the user specified by "locuser" */
> - setegid ((gid_t) pwd->pw_gid);
> - setgid ((gid_t) pwd->pw_gid);
> + if (setegid ((gid_t) pwd->pw_gid) == -1)
> + {
> + rshd_error ("Cannot drop privileges (setegid() failed)\n");
> + exit (EXIT_FAILURE);
> + }
> +
> + if (setgid ((gid_t) pwd->pw_gid) == -1)
> + {
> + rshd_error ("Cannot drop privileges (setgid() failed)\n");
> + exit (EXIT_FAILURE);
> + }
> +
> #ifdef HAVE_INITGROUPS
> initgroups (pwd->pw_name, pwd->pw_gid); /* BSD groups */
> #endif
> @@ -1870,7 +1880,11 @@ doit (int sockfd, struct sockaddr *fromp, socklen_t
> fromlen)
> }
> #endif /* WITH_PAM */
>
> - setuid ((uid_t) pwd->pw_uid);
> + if (setuid ((uid_t) pwd->pw_uid) == -1)
> + {
> + rshd_error ("Cannot drop privileges (setuid() failed)\n");
> + exit (EXIT_FAILURE);
> + }
>
> /* We'll execute the client's command in the home directory
> * of locuser. Note, that the chdir must be executed after
> diff --git a/src/uucpd.c b/src/uucpd.c
> index 107589e..29cfce3 100644
> --- a/src/uucpd.c
> +++ b/src/uucpd.c
> @@ -252,7 +252,12 @@ doit (struct sockaddr *sap, socklen_t salen)
> snprintf (Username, sizeof (Username), "USER=%s", user);
> snprintf (Logname, sizeof (Logname), "LOGNAME=%s", user);
> dologin (pw, sap, salen);
> - setgid (pw->pw_gid);
> +
> + if (setgid (pw->pw_gid) == -1)
> + {
> + fprintf (stderr, "setgid() failed");
> + return;
> + }
> #ifdef HAVE_INITGROUPS
> initgroups (pw->pw_name, pw->pw_gid);
> #endif
> @@ -261,7 +266,13 @@ doit (struct sockaddr *sap, socklen_t salen)
> fprintf (stderr, "Login incorrect.");
> return;
> }
> - setuid (pw->pw_uid);
> +
> + if (setuid (pw->pw_uid) == -1)
> + {
> + fprintf (stderr, "setuid() failed");
> + return;
> + }
> +
> execl (uucico_location, "uucico", NULL);
> perror ("uucico server: execl");
> }
> --
> 2.35.1
>
> --
> Jeffrey BENCTEUX
>
