bug-inetutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

setuid/setgid return values not checked in rlogin, rsh, rshd and uucpd


From: Jeffrey
Subject: setuid/setgid return values not checked in rlogin, rsh, rshd and uucpd
Date: Sat, 1 Jul 2023 10:30:57 +0200

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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]