Re: setuid/setgid return values not checked in rlogin, rsh, rshd and uuc

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 <jeffbencteux@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 <jeffbencteux@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 <jeffbencteux@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

From:	Jeffrey
Subject:	Re: setuid/setgid return values not checked in rlogin, rsh, rshd and uucpd
Date:	Sat, 1 Jul 2023 11:39:42 +0200