From 358750fa3772bf3878cf44e2760e3cf7ea8c33dd Mon Sep 17 00:00:00 2001 From: Andy Fiddaman Date: Wed, 21 May 2025 14:29:27 +0000 Subject: [PATCH] screen: fixes for CVE-2025-4680[245] --- build/screen/build.sh | 3 +- ...nt-temporary-0666-mode-on-PTYs-to-fi.patch | 140 ++++++++++++++++++ ...ence-test-information-leaks-to-fix-C.patch | 123 +++++++++++++++ ...end-signals-with-root-privileges-to-.patch | 117 +++++++++++++++ build/screen/patches/series | 3 + 5 files changed, 385 insertions(+), 1 deletion(-) create mode 100644 build/screen/patches/0001-attacher.c-prevent-temporary-0666-mode-on-PTYs-to-fi.patch create mode 100644 build/screen/patches/0002-Avoid-file-existence-test-information-leaks-to-fix-C.patch create mode 100644 build/screen/patches/0003-socket.c-don-t-send-signals-with-root-privileges-to-.patch diff --git a/build/screen/build.sh b/build/screen/build.sh index 99b0698be..ed55e34fc 100755 --- a/build/screen/build.sh +++ b/build/screen/build.sh @@ -13,12 +13,13 @@ # }}} # # Copyright 2017 OmniTI Computer Consulting, Inc. All rights reserved. -# Copyright 2024 OmniOS Community Edition (OmniOSce) Association. +# Copyright 2025 OmniOS Community Edition (OmniOSce) Association. . ../../lib/build.sh PROG=screen VER=4.9.1 +DASHREV=1 PKG=terminal/screen SUMMARY="GNU Screen terminal multiplexer" DESC="A full-screen window manager that multiplexes a physical " diff --git a/build/screen/patches/0001-attacher.c-prevent-temporary-0666-mode-on-PTYs-to-fi.patch b/build/screen/patches/0001-attacher.c-prevent-temporary-0666-mode-on-PTYs-to-fi.patch new file mode 100644 index 000000000..bdf411515 --- /dev/null +++ b/build/screen/patches/0001-attacher.c-prevent-temporary-0666-mode-on-PTYs-to-fi.patch @@ -0,0 +1,140 @@ +From 089b97333844e05a4693a3ee726ac6bdb2d6bf59 Mon Sep 17 00:00:00 2001 +From: Matthias Gerstner +Date: Wed, 7 May 2025 12:59:05 +0200 +Subject: [PATCH 1/3] attacher.c: prevent temporary 0666 mode on PTYs to fix + CVE-2025-46802 + +This temporary chmod of the PTY to mode 0666 is most likely a remnant of +past times, before the PTY file descriptor was passed to the target +session via the UNIX domain socket. + +This chmod() causes a race condition during which any other user in the +system can open the PTY for reading and writing, and thus allows PTY +hijacking. + +Simply remove this logic completely. +--- + attacher.c | 27 --------------------------- + screen.c | 19 ------------------- + 2 files changed, 46 deletions(-) + +diff --git a/attacher.c b/attacher.c +index c35ae7a..16b151e 100644 +--- a/attacher.c ++++ b/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; + } +diff --git a/screen.c b/screen.c +index 7653cd1..1a23e1a 100644 +--- a/screen.c ++++ b/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) +@@ -2170,20 +2165,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); + } + +-- +2.49.0 + diff --git a/build/screen/patches/0002-Avoid-file-existence-test-information-leaks-to-fix-C.patch b/build/screen/patches/0002-Avoid-file-existence-test-information-leaks-to-fix-C.patch new file mode 100644 index 000000000..6a22e2d8f --- /dev/null +++ b/build/screen/patches/0002-Avoid-file-existence-test-information-leaks-to-fix-C.patch @@ -0,0 +1,123 @@ +From 1e2a17d8437e5f284daeae54f0b4722b48eb0e30 Mon Sep 17 00:00:00 2001 +From: Matthias Gerstner +Date: Wed, 7 May 2025 13:12:53 +0200 +Subject: [PATCH 2/3] Avoid file existence test information leaks to fix + CVE-2025-46804 + +In setuid-root context the current error messages give away whether +certain paths not accessible by the real user exist and what type they +have. To prevent this only output generic error messages in setuid-root +context. + +In some situations, when an error is pertaining a directory and the +directory is owner by the real user then we can still output more +detailed diagnostics. + +This change can lead to less helpful error messages when Screen is +install setuid-root. More complex changes would be needed to avoid this +(e.g. only open the `SocketPath` with raised privileges when +multi-attach is requested). + +There might still be lingering some code paths that allow such +information leaks, since `SocketPath` is a global variable that is used +across the code base. The majority of issues should be caught with this +fix, however. +--- + screen.c | 43 +++++++++++++++++++++++++++++++++---------- + socket.c | 9 +++++++-- + 2 files changed, 40 insertions(+), 12 deletions(-) + +diff --git a/screen.c b/screen.c +index 1a23e1a..024239d 100644 +--- a/screen.c ++++ b/screen.c +@@ -1122,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 +@@ -1145,8 +1158,13 @@ int main(int ac, char** av) + #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; +@@ -1184,8 +1202,13 @@ 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); + } +diff --git a/socket.c b/socket.c +index 54d8cb8..6c3502f 100644 +--- a/socket.c ++++ b/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; +-- +2.49.0 + diff --git a/build/screen/patches/0003-socket.c-don-t-send-signals-with-root-privileges-to-.patch b/build/screen/patches/0003-socket.c-don-t-send-signals-with-root-privileges-to-.patch new file mode 100644 index 000000000..32bd8a2d5 --- /dev/null +++ b/build/screen/patches/0003-socket.c-don-t-send-signals-with-root-privileges-to-.patch @@ -0,0 +1,117 @@ +From f5d0230db7f61e1494573cbc846177464eb322d4 Mon Sep 17 00:00:00 2001 +From: Matthias Gerstner +Date: Wed, 7 May 2025 15:03:38 +0200 +Subject: [PATCH 3/3] socket.c: don't send signals with root privileges to fix + CVE-2025-46805 + +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. +--- + socket.c | 21 +++++++++++++-------- + 1 file changed, 13 insertions(+), 8 deletions(-) + +diff --git a/socket.c b/socket.c +index 6c3502f..d6621fa 100644 +--- a/socket.c ++++ b/socket.c +@@ -831,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" +@@ -916,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 +@@ -940,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) +@@ -965,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; + } +@@ -976,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; + } +@@ -1294,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 */ +-- +2.49.0 + diff --git a/build/screen/patches/series b/build/screen/patches/series index ca6f1d8ac..679d237e1 100644 --- a/build/screen/patches/series +++ b/build/screen/patches/series @@ -2,3 +2,6 @@ Makefile.in.patch ncurses.patch unneededlibs.patch gcc14.patch +0001-attacher.c-prevent-temporary-0666-mode-on-PTYs-to-fi.patch +0002-Avoid-file-existence-test-information-leaks-to-fix-C.patch +0003-socket.c-don-t-send-signals-with-root-privileges-to-.patch