fix CVE-2025-46805: socket.c - don't send signals with root privileges

The CheckPid() function was introduced to address CVE-2023-24626, to
prevent sending SIGCONT and SIGHUP to arbitrary PIDs in the system. This
fix still suffers from a TOCTOU race condition. The client can replace
itself by a privileged process, or try to cycle PIDs until a privileged
process receives the original PID.

To prevent this, always send signals using the real privileges. Keep
CheckPid() for error diagnostics. If sending the actual signal fails
later on then there will be no more error reporting.

It seems the original bugfix already introduced a regression when
attaching to another's user session that is not owned by root. In this
case the target sessions runs with real uid X, while for sending a
signal to the `pid` provided by the client real uid Y (or root
privileges) are required.

This is hard to properly fix without this regression. On Linux pidfds
could be used to allow safely sending signals to other PIDs as root
without involving race conditions. In this case the client PID should
also be obtained via the UNIX domain socket's SO_PEERCRED option,
though.

src/attacher.c

@@ -73,7 +73,6 @@ extern int MasterPid, attach_fd;
 #ifdef MULTIUSER
 extern char *multi;
 extern int multiattach, multi_uid, own_uid;
-extern int tty_mode, tty_oldmode;
 # ifndef USE_SETEUID
 static int multipipe[2];
 # endif
@@ -160,9 +159,6 @@ int how;
 
       if (pipe(multipipe))
 	Panic(errno, "pipe");
-      if (chmod(attach_tty, 0666))
-	Panic(errno, "chmod %s", attach_tty);
-      tty_oldmode = tty_mode;
       eff_uid = -1;	/* make UserContext fork */
       real_uid = multi_uid;
       if ((ret = UserContext()) <= 0)
@@ -174,11 +170,6 @@ int how;
 	    Panic(errno, "UserContext");
 	  close(multipipe[1]);
 	  read(multipipe[0], &dummy, 1);
-	  if (tty_oldmode >= 0)
-	    {
-	      chmod(attach_tty, tty_oldmode);
-	      tty_oldmode = -1;
-	    }
 	  ret = UserStatus();
 #ifdef LOCK
 	  if (ret == SIG_LOCK)
@@ -224,9 +215,6 @@ int how;
       xseteuid(multi_uid);
       xseteuid(own_uid);
 #endif
-      if (chmod(attach_tty, 0666))
-	Panic(errno, "chmod %s", attach_tty);
-      tty_oldmode = tty_mode;
     }
 # endif /* USE_SETEUID */
 #endif /* MULTIUSER */
@@ -423,13 +411,6 @@ int how;
       ContinuePlease = 0;
 # ifndef USE_SETEUID
       close(multipipe[1]);
-# else
-      xseteuid(own_uid);
-      if (tty_oldmode >= 0)
-        if (chmod(attach_tty, tty_oldmode))
-          Panic(errno, "chmod %s", attach_tty);
-      tty_oldmode = -1;
-      xseteuid(real_uid);
 # endif
     }
 #endif
@@ -505,14 +486,6 @@ AttacherFinit SIGDEFARG
 	  close(s);
 	}
     }
-#ifdef MULTIUSER
-  if (tty_oldmode >= 0)
-    {
-      if (setuid(own_uid))
-        Panic(errno, "setuid");
-      chmod(attach_tty, tty_oldmode);
-    }
-#endif
   exit(0);
   SIGRETURN;
 }

src/screen.c

@@ -230,8 +230,6 @@ char *multi_home;
 int multi_uid;
 int own_uid;
 int multiattach;
-int tty_mode;
-int tty_oldmode = -1;
 #endif
 
 char HostName[MAXSTR];
@@ -1009,9 +1007,6 @@ int main(int ac, char** av)
 
     /* ttyname implies isatty */
     SetTtyname(true, &st);
-#ifdef MULTIUSER
-    tty_mode = (int)st.st_mode & 0777;
-#endif
 
     fl = fcntl(0, F_GETFL, 0);
     if (fl != -1 && (fl & (O_RDWR|O_RDONLY|O_WRONLY)) == O_RDWR)
@@ -1127,15 +1122,28 @@ int main(int ac, char** av)
 #endif
   }
 
-  if (stat(SockPath, &st) == -1)
-    Panic(errno, "Cannot access %s", SockPath);
-  else
-    if (!S_ISDIR(st.st_mode))
+  if (stat(SockPath, &st) == -1) {
+    if (eff_uid == real_uid) {
+      Panic(errno, "Cannot access %s", SockPath);
+    } else {
+      Panic(0, "Error accessing %s", SockPath);
+    }
+  } else if (!S_ISDIR(st.st_mode)) {
+    if (eff_uid == real_uid || st.st_uid == real_uid) {
       Panic(0, "%s is not a directory.", SockPath);
+    } else {
+      Panic(0, "Error accessing %s", SockPath);
+    }
+  }
 #ifdef MULTIUSER
   if (multi) {
-    if ((int)st.st_uid != multi_uid)
-      Panic(0, "%s is not the owner of %s.", multi, SockPath);
+    if ((int)st.st_uid != multi_uid) {
+      if (eff_uid == real_uid || st.st_uid == real_uid) {
+        Panic(0, "%s is not the owner of %s.", multi, SockPath);
+      } else {
+        Panic(0, "Error accessing %s", SockPath);
+      }
+    }
   }
   else
 #endif
@@ -1149,9 +1157,13 @@ int main(int ac, char** av)
       Panic(0, "You are not the owner of %s.", SockPath);
 #endif
   }
-
-  if ((st.st_mode & 0777) != 0700)
-    Panic(0, "Directory %s must have mode 700.", SockPath);
+  if ((st.st_mode & 0777) != 0700) {
+    if (eff_uid == real_uid || st.st_uid == real_uid) {
+      Panic(0, "Directory %s must have mode 700.", SockPath);
+    } else {
+      Panic(0, "Error accessing %s", SockPath);
+    }
+  }
   if (SockMatch && index(SockMatch, '/'))
     Panic(0, "Bad session name '%s'", SockMatch);
   SockName = SockPath + strlen(SockPath) + 1;
@@ -1189,8 +1201,14 @@ int main(int ac, char** av)
       else
         exit(9 + (fo || oth ? 1 : 0) + fo);
     }
-    if (fo == 0)
-      Panic(0, "No Sockets found in %s.\n", SockPath);
+    if (fo == 0) {
+      if (eff_uid == real_uid || st.st_uid == real_uid) {
+        Panic(0, "No Sockets found in %s.\n", SockPath);
+      } else {
+        Panic(0, "Error accessing %s", SockPath);
+      }
+    }
+
     Msg(0, "%d Socket%s in %s.", fo, fo > 1 ? "s" : "", SockPath);
     eexit(0);
   }
@@ -2170,20 +2188,6 @@ DEFINE_VARARGS_FN(Panic)
       if (D_userpid)
         Kill(D_userpid, SIG_BYE);
     }
-#ifdef MULTIUSER
-  if (tty_oldmode >= 0) {
-
-# ifdef USE_SETEUID
-    if (setuid(own_uid))
-      xseteuid(own_uid);	/* may be a loop. sigh. */
-# else
-      setuid(own_uid);
-# endif
-
-    debug1("Panic: changing back modes from %s\n", attach_tty);
-    chmod(attach_tty, tty_oldmode);
-  }
-#endif
   eexit(1);
 }
 

src/socket.c

@@ -169,8 +169,13 @@ bool *is_sock;
   xsetegid(real_gid);
 #endif
 
-  if ((dirp = opendir(SockPath)) == 0)
-    Panic(errno, "Cannot opendir %s", SockPath);
+  if ((dirp = opendir(SockPath)) == 0) {
+    if (eff_uid == real_uid) {
+      Panic(errno, "Cannot opendir %s", SockPath);
+    } else {
+      Panic(0, "Error accessing %s", SockPath);
+    }
+  }
 
   slist = 0;
   slisttail = &slist;
@@ -826,6 +831,11 @@ int pid;
   return UserStatus();
 }
 
+static void KillUnpriv(pid_t pid, int sig) {
+    UserContext();
+    UserReturn(kill(pid, sig));
+}
+
 #ifdef hpux
 /*
  * From: "F. K. Bruner" <napalm@ugcs.caltech.edu>
@@ -911,14 +921,14 @@ struct win *wi;
             {
 	      Msg(errno, "Could not perform necessary sanity checks on pts device.");
 	      close(i);
-	      Kill(pid, SIG_BYE);
+	      KillUnpriv(pid, SIG_BYE);
 	      return -1;
             }
           if (strcmp(ttyname_in_ns, m->m_tty))
             {
 	      Msg(errno, "Attach: passed fd does not match tty: %s - %s!", ttyname_in_ns, m->m_tty[0] != '\0' ? m->m_tty : "(null)");
 	      close(i);
-	      Kill(pid, SIG_BYE);
+	      KillUnpriv(pid, SIG_BYE);
 	      return -1;
 	    }
 	  /* m->m_tty so far contains the actual name of the pts device in the
@@ -935,19 +945,19 @@ struct win *wi;
 	{
 	  Msg(errno, "Attach: passed fd does not match tty: %s - %s!", m->m_tty, myttyname ? myttyname : "NULL");
 	  close(i);
-	  Kill(pid, SIG_BYE);
+	  KillUnpriv(pid, SIG_BYE);
 	  return -1;
 	}
     }
   else if ((i = secopen(m->m_tty, O_RDWR | O_NONBLOCK, 0)) < 0)
     {
       Msg(errno, "Attach: Could not open %s!", m->m_tty);
-      Kill(pid, SIG_BYE);
+      KillUnpriv(pid, SIG_BYE);
       return -1;
     }
 #ifdef MULTIUSER
   if (attach)
-    Kill(pid, SIGCONT);
+    KillUnpriv(pid, SIGCONT);
 #endif
 
 #if defined(ultrix) || defined(pyr) || defined(NeXT)
@@ -960,7 +970,7 @@ struct win *wi;
 	{
 	  write(i, "Attaching from inside of screen?\n", 33);
 	  close(i);
-	  Kill(pid, SIG_BYE);
+	  KillUnpriv(pid, SIG_BYE);
 	  Msg(0, "Attach msg ignored: coming from inside.");
 	  return -1;
 	}
@@ -971,7 +981,7 @@ struct win *wi;
 	  {
 	      write(i, "Access to session denied.\n", 26);
 	      close(i);
-	      Kill(pid, SIG_BYE);
+	      KillUnpriv(pid, SIG_BYE);
 	      Msg(0, "Attach: access denied for user %s.", user);
 	      return -1;
 	  }
@@ -1289,7 +1299,7 @@ ReceiveMsg()
             Msg(0, "Query attempt with bad pid(%d)!", m.m.command.apid);
           }
           else {
-            Kill(m.m.command.apid,
+            KillUnpriv(m.m.command.apid,
                (queryflag >= 0)
                    ? SIGCONT
                    : SIG_BYE); /* Send SIG_BYE if an error happened */