Bug#1105892: [pre-approval request] unblock: screen/4.9.1-3 (was: screen/4.9.1-2.1)
Axel Beckert wrote:
> Full debdiff attached again, sole changes compared to Salvatore's
> debdiff are in debian/changelog (NMU → MU).
Forgot the attachment. Here is it.
Regards, Axel
--
,''`. | Axel Beckert <abe@debian.org>, https://people.debian.org/~abe/
: :' : | Debian Developer, ftp.ch.debian.org Admin
`. `' | 4096R: 2517 B724 C5F6 CA99 5329 6E61 2FF9 CD59 6126 16B5
`- | 1024D: F067 EA27 26B9 C3FC 1486 202E C09E 1D89 9593 0EDE
diff -Nru screen-4.9.1/debian/changelog screen-4.9.1/debian/changelog
--- screen-4.9.1/debian/changelog 2025-05-10 22:28:23.000000000 +0200
+++ screen-4.9.1/debian/changelog 2025-05-19 00:42:42.000000000 +0200
@@ -1,3 +1,13 @@
+screen (4.9.1-3) unstable; urgency=medium
+
+ [ Salvatore Bonaccorso ]
+ * attacher.c - prevent temporary 0666 mode on PTYs (CVE-2025-46802)
+ (Closes: #1105191)
+ * avoid file existence test information leaks (CVE-2025-46804)
+ * socket.c - don't send signals with root privileges (CVE-2025-46805)
+
+ -- Axel Beckert <abe@debian.org> Mon, 19 May 2025 00:42:42 +0200
+
screen (4.9.1-2) unstable; urgency=medium
* Team upload (debian/ namespace).
diff -Nru screen-4.9.1/debian/patches/fix-CVE-2025-46802-attacher.c-prevent-temporary-0666.patch screen-4.9.1/debian/patches/fix-CVE-2025-46802-attacher.c-prevent-temporary-0666.patch
--- screen-4.9.1/debian/patches/fix-CVE-2025-46802-attacher.c-prevent-temporary-0666.patch 1970-01-01 01:00:00.000000000 +0100
+++ screen-4.9.1/debian/patches/fix-CVE-2025-46802-attacher.c-prevent-temporary-0666.patch 2025-05-19 00:26:08.000000000 +0200
@@ -0,0 +1,134 @@
+From: Matthias Gerstner <matthias.gerstner@suse.de>
+Date: Mon, 12 May 2025 15:15:38 +0200
+Subject: fix CVE-2025-46802: attacher.c - prevent temporary 0666 mode on PTYs
+Origin: https://git.savannah.gnu.org/cgit/screen.git/commit/?id=049b26b22e197ba3be9c46e5c193032e01a4724a
+Bug-Debian: https://bugs.debian.org/1105191
+Bug-Debian-Security: https://security-tracker.debian.org/tracker/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.
+---
+ src/attacher.c | 27 ---------------------------
+ src/screen.c | 19 -------------------
+ 2 files changed, 46 deletions(-)
+
+--- 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;
+ }
+--- 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);
+ }
+
diff -Nru screen-4.9.1/debian/patches/fix-CVE-2025-46804-avoid-file-existence-test-informa.patch screen-4.9.1/debian/patches/fix-CVE-2025-46804-avoid-file-existence-test-informa.patch
--- screen-4.9.1/debian/patches/fix-CVE-2025-46804-avoid-file-existence-test-informa.patch 1970-01-01 01:00:00.000000000 +0100
+++ screen-4.9.1/debian/patches/fix-CVE-2025-46804-avoid-file-existence-test-informa.patch 2025-05-19 00:26:08.000000000 +0200
@@ -0,0 +1,118 @@
+From: Matthias Gerstner <matthias.gerstner@suse.de>
+Date: Mon, 12 May 2025 15:26:11 +0200
+Subject: fix CVE-2025-46804: avoid file existence test information leaks
+Origin: https://git.savannah.gnu.org/cgit/screen.git/commit/?id=e0eef5aac453fa98a2664416a56c50ad1d00cb30
+Bug-Debian-Security: https://security-tracker.debian.org/tracker/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.
+---
+ src/screen.c | 45 ++++++++++++++++++++++++++++++++++-----------
+ src/socket.c | 9 +++++++--
+ 2 files changed, 41 insertions(+), 13 deletions(-)
+
+--- 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
+@@ -1144,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;
+@@ -1184,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);
+ }
+--- a/socket.c
++++ b/socket.c
+@@ -171,8 +171,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 = schosen = NULL;
+ slisttail = &slist;
diff -Nru screen-4.9.1/debian/patches/fix-CVE-2025-46805-socket.c-don-t-send-signals-with-.patch screen-4.9.1/debian/patches/fix-CVE-2025-46805-socket.c-don-t-send-signals-with-.patch
--- screen-4.9.1/debian/patches/fix-CVE-2025-46805-socket.c-don-t-send-signals-with-.patch 1970-01-01 01:00:00.000000000 +0100
+++ screen-4.9.1/debian/patches/fix-CVE-2025-46805-socket.c-don-t-send-signals-with-.patch 2025-05-19 00:26:08.000000000 +0200
@@ -0,0 +1,113 @@
+From: Matthias Gerstner <matthias.gerstner@suse.de>
+Date: Mon, 12 May 2025 15:38:19 +0200
+Subject: fix CVE-2025-46805: socket.c - don't send signals with root
+ privileges
+Origin: https://git.savannah.gnu.org/cgit/screen.git/commit/?id=161f85b98b7e1d5e4893aeed20f4cdb5e3dfaaa4
+Bug-Debian-Security: https://security-tracker.debian.org/tracker/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.
+---
+ src/socket.c | 21 +++++++++++++--------
+ 1 file changed, 13 insertions(+), 8 deletions(-)
+
+--- a/socket.c
++++ b/socket.c
+@@ -832,6 +832,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>
+@@ -917,14 +922,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
+@@ -941,19 +946,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)
+@@ -966,7 +971,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;
+ }
+@@ -977,7 +982,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;
+ }
+@@ -1295,7 +1300,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 */
diff -Nru screen-4.9.1/debian/patches/series screen-4.9.1/debian/patches/series
--- screen-4.9.1/debian/patches/series 2025-05-10 22:23:13.000000000 +0200
+++ screen-4.9.1/debian/patches/series 2025-05-19 00:26:08.000000000 +0200
@@ -15,3 +15,6 @@
81_session_creation_util.patch
82_session_creation_core.patch
85_bracketed-paste-patch-by-Unit193_dpaste.com_5KJ572GZM.patch
+fix-CVE-2025-46802-attacher.c-prevent-temporary-0666.patch
+fix-CVE-2025-46804-avoid-file-existence-test-informa.patch
+fix-CVE-2025-46805-socket.c-don-t-send-signals-with-.patch
Reply to: