From dd50014055a36969153e83cf3675d9559fa6a200 Mon Sep 17 00:00:00 2001
From: Tobias Stoeckmann <tobias@stoeckmann.org>
Date: Sat, 2 Jul 2016 18:11:09 +0200
Subject: [PATCH] Fixed signal races in shadow tools.
Some of the supplied tools use functions which are not signal-safe.
Most of the times it's exit() vs. _exit().
In other times it's how the standard output or standard error is
handled. FILE-related functions shall be avoided, therefore I replaced
them with write().
Also there is no need to call closelog(). At worst, it allows to
trigger a deadlock by issuing different signal types at bad timings.
But as these fixes are about race conditions, expect bad timings in
general for these bugs to be triggered. :)
---
src/expiry.c | 2 +-
src/gpasswd.c | 5 ++---
src/login.c | 11 +++++++----
src/su.c | 15 +++++++++------
src/sulogin.c | 2 +-
5 files changed, 20 insertions(+), 15 deletions(-)
diff --git a/src/expiry.c b/src/expiry.c
index 4ae47703..41add942 100644
--- a/src/expiry.c
+++ b/src/expiry.c
@@ -58,7 +58,7 @@ static void process_flags (int argc, char **argv);
*/
static RETSIGTYPE catch_signals (unused int sig)
{
- exit (10);
+ _exit (10);
}
/*
diff --git a/src/gpasswd.c b/src/gpasswd.c
index 27ad9599..c4a492b1 100644
--- a/src/gpasswd.c
+++ b/src/gpasswd.c
@@ -169,9 +169,8 @@ static RETSIGTYPE catch_signals (int killed)
}
if (0 != killed) {
- (void) putchar ('\n');
- (void) fflush (stdout);
- exit (killed);
+ (void) write (STDOUT_FILENO, "\n", 1);
+ _exit (killed);
}
}
diff --git a/src/login.c b/src/login.c
index d610844d..2d2e704e 100644
--- a/src/login.c
+++ b/src/login.c
@@ -103,7 +103,7 @@ static bool hflg = false;
static bool preauth_flag = false;
static bool amroot;
-static unsigned int timeout;
+static char tmsg[256];
/*
* External identifiers.
@@ -416,8 +416,8 @@ static void init_env (void)
static RETSIGTYPE alarm_handler (unused int sig)
{
- fprintf (stderr, _("\nLogin timed out after %u seconds.\n"), timeout);
- exit (0);
+ write (STDERR_FILENO, tmsg, strlen (tmsg));
+ _exit (0);
}
#ifdef USE_PAM
@@ -532,6 +532,7 @@ int main (int argc, char **argv)
bool is_console;
#endif
int err;
+ unsigned int timeout;
const char *cp;
const char *tmp;
char fromhost[512];
@@ -698,8 +699,10 @@ int main (int argc, char **argv)
top:
/* only allow ALARM sec. for login */
- (void) signal (SIGALRM, alarm_handler);
timeout = getdef_unum ("LOGIN_TIMEOUT", ALARM);
+ snprintf (tmsg, sizeof tmsg,
+ _("\nLogin timed out after %u seconds.\n"), timeout);
+ (void) signal (SIGALRM, alarm_handler);
if (timeout > 0) {
(void) alarm (timeout);
}
diff --git a/src/su.c b/src/su.c
index 37042172..2c27e98e 100644
--- a/src/su.c
+++ b/src/su.c
@@ -105,6 +105,8 @@ static char caller_name[BUFSIZ];
static bool change_environment = true;
#ifdef USE_PAM
+static char kill_msg[256];
+static char wait_msg[256];
static pam_handle_t *pamh = NULL;
static int caught = 0;
/* PID of the child, in case it needs to be killed */
@@ -161,8 +163,7 @@ static RETSIGTYPE die (int killed)
}
if (killed != 0) {
- closelog ();
- exit (128+killed);
+ _exit (128+killed);
}
}
@@ -182,12 +183,11 @@ static RETSIGTYPE kill_child (int unused(s))
{
if (0 != pid_child) {
(void) kill (-pid_child, SIGKILL);
- (void) fputs (_(" ...killed.\n"), stderr);
+ (void) write (STDERR_FILENO, kill_msg, strlen (kill_msg));
} else {
- (void) fputs (_(" ...waiting for child to terminate.\n"),
- stderr);
+ (void) write (STDERR_FILENO, wait_msg, strlen (wait_msg));
}
- exit (255);
+ _exit (255);
}
#endif /* USE_PAM */
@@ -373,6 +373,9 @@ static void prepare_pam_close_session (void)
stderr);
(void) kill (-pid_child, caught);
+ snprintf (kill_msg, _(" ...killed.\n"));
+ snprintf (wait_msg, _(" ...waiting for child to terminate.\n"));
+
(void) signal (SIGALRM, kill_child);
(void) alarm (2);
diff --git a/src/sulogin.c b/src/sulogin.c
index ccbf2c5d..4264099b 100644
--- a/src/sulogin.c
+++ b/src/sulogin.c
@@ -70,7 +70,7 @@ static RETSIGTYPE catch_signals (int);
static RETSIGTYPE catch_signals (unused int sig)
{
- exit (1);
+ _exit (1);
}
/*