115 lines
3.7 KiB
Diff
115 lines
3.7 KiB
Diff
|
|
From 161f85b98b7e1d5e4893aeed20f4cdb5e3dfaaa4 Mon Sep 17 00:00:00 2001
|
||
|
|
From: Matthias Gerstner <matthias.gerstner@suse.de>
|
||
|
|
Date: Mon, 12 May 2025 15:38:19 +0200
|
||
|
|
Subject: [PATCH 3/3] 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.
|
||
|
|
---
|
||
|
|
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" <napalm@ugcs.caltech.edu>
|
||
|
|
@@ -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 */
|