su: properly clear child PID

If su is compiled with PAM support, it is possible for any local user
to send SIGKILL to other processes with root privileges. There are
only two conditions. First, the user must be able to perform su with
a successful login. This does NOT have to be the root user, even using
su with the same id is enough, e.g. "su $(whoami)". Second, SIGKILL
can only be sent to processes which were executed after the su process.
It is not possible to send SIGKILL to processes which were already
running. I consider this as a security vulnerability, because I was
able to write a proof of concept which unlocked a screen saver of
another user this way.

diff --git a/src/su.c b/src/su.c
index f20d230ab5f31adae19f811cd468e91cb0157d0c..d86aa86c46fb0fad1c606165dc53d701605d8f45 100644 (file)
--- a/src/su.c
+++ b/src/su.c
@@ -379,11 +379,13 @@ static void prepare_pam_close_session (void)
                                /* wake child when resumed */
                                kill (pid, SIGCONT);
                                stop = false;
+                       } else {
+                               pid_child = 0;
                        }
                } while (!stop);
        }
 
-       if (0 != caught) {
+       if (0 != caught && 0 != pid_child) {
                (void) fputs ("\n", stderr);
                (void) fputs (_("Session terminated, terminating shell..."),
                              stderr);
@@ -393,9 +395,22 @@ static void prepare_pam_close_session (void)
                snprintf (wait_msg, sizeof wait_msg, _(" ...waiting for child to terminate.\n"));
 
                (void) signal (SIGALRM, kill_child);
+               (void) signal (SIGCHLD, catch_signals);
                (void) alarm (2);
 
-               (void) wait (&status);
+               sigemptyset (&ourset);
+               if ((sigaddset (&ourset, SIGALRM) != 0)
+                   || (sigprocmask (SIG_BLOCK, &ourset, NULL) != 0)) {
+                       fprintf (stderr, _("%s: signal masking malfunction\n"), Prog);
+                       kill_child (0);
+               } else {
+                       while (0 == waitpid (pid_child, &status, WNOHANG)) {
+                               sigsuspend (&ourset);
+                       }
+                       pid_child = 0;
+                       (void) sigprocmask (SIG_UNBLOCK, &ourset, NULL);
+               }
+
                (void) fputs (_(" ...terminated.\n"), stderr);
        }