[Pkg-shadow-commits] r3335 - in upstream/trunk: . src

Nicolas FRANÇOIS nekral-guest at alioth.debian.org
Sun Jun 5 14:41:16 UTC 2011


Author: nekral-guest
Date: 2011-06-05 14:41:15 +0000 (Sun, 05 Jun 2011)
New Revision: 3335

Modified:
   upstream/trunk/ChangeLog
   upstream/trunk/NEWS
   upstream/trunk/src/su.c
Log:
	* NEWS, src/su.c: Do not forward the controlling terminal to
	commands executed with -c. This prevents tty hijacking which could
	lead to execution with the caller's privileges. This required to
	forward signals from the terminal (SIGINT, SIGQUIT, SIGTSTP) to
	the executed command.


Modified: upstream/trunk/ChangeLog
===================================================================
--- upstream/trunk/ChangeLog	2011-06-05 12:23:59 UTC (rev 3334)
+++ upstream/trunk/ChangeLog	2011-06-05 14:41:15 UTC (rev 3335)
@@ -1,5 +1,13 @@
 2011-06-05  Nicolas François  <nicolas.francois at centraliens.net>
 
+	* NEWS, src/su.c: Do not forward the controlling terminal to
+	commands executed with -c. This prevents tty hijacking which could
+	lead to execution with the caller's privileges. This required to
+	forward signals from the terminal (SIGINT, SIGQUIT, SIGTSTP) to
+	the executed command.
+
+2011-06-05  Nicolas François  <nicolas.francois at centraliens.net>
+
 	* NEWS, src/userdel.c: Do not remove a group with the same name as
 	the user (usergroup) if this group isn't the user's primary group.
 

Modified: upstream/trunk/NEWS
===================================================================
--- upstream/trunk/NEWS	2011-06-05 12:23:59 UTC (rev 3334)
+++ upstream/trunk/NEWS	2011-06-05 14:41:15 UTC (rev 3335)
@@ -2,7 +2,11 @@
 
 shadow-4.1.4.3 -> shadow-4.1.5					UNRELEASED
 
-- general
+*** security
+  * su -c could be abused by the executed command to invoke commands with
+    the caller privileges. See below.
+
+*** general
   * report usage error to stderr, but report usage help to stdout (and return
     zero) when explicitly requested (e.g. with --help).
   * initial support for tcb (http://openwall.com/tcb/) for useradd,
@@ -39,6 +43,9 @@
     list of TTYs.
   * Fixed warning and support for CONSOLE_GROUPS for users member of more
     than 16 groups.
+  * Do not forward the controlling terminal to commands executed with -c.
+    This prevents tty hijacking which could lead to execution with the
+    caller's privileges.
 - newgrp, sg, groupmems
   * Fix parsing of gshadow entries.
 - useradd
@@ -59,7 +66,7 @@
 
 shadow-4.1.4.2 -> shadow-4.1.4.3						2011-02-15
 
-*** security:
+*** security
 - CVE-2011-0721: An insufficient input sanitation in chfn can be exploited
   to create users or groups in a NIS environment.
 

Modified: upstream/trunk/src/su.c
===================================================================
--- upstream/trunk/src/su.c	2011-06-05 12:23:59 UTC (rev 3334)
+++ upstream/trunk/src/su.c	2011-06-05 14:41:15 UTC (rev 3335)
@@ -88,7 +88,7 @@
 
 #ifdef USE_PAM
 static pam_handle_t *pamh = NULL;
-static bool caught = false;
+static int caught = 0;
 /* PID of the child, in case it needs to be killed */
 static pid_t pid_child = 0;
 #endif
@@ -235,9 +235,9 @@
 
 #ifdef USE_PAM
 /* Signal handler for parent process later */
-static void catch_signals (unused int sig)
+static void catch_signals (int sig)
 {
-	caught = true;
+	caught = sig;
 }
 
 /* This I ripped out of su.c from sh-utils after the Mandrake pam patch
@@ -264,6 +264,11 @@
 		if (doshell) {
 			(void) shell (shellstr, (char *) args[0], envp);
 		} else {
+			/* There is no need for a controlling terminal.
+			 * This avoids the callee to inject commands on
+			 * the caller's tty. */
+			(void) setsid ();
+
 			execve_shell (shellstr, (char **) args, envp);
 		}
 
@@ -283,9 +288,9 @@
 		(void) fprintf (stderr,
 		                _("%s: signal malfunction\n"),
 		                Prog);
-		caught = true;
+		caught = SIGTERM;
 	}
-	if (!caught) {
+	if (0 == caught) {
 		struct sigaction action;
 
 		action.sa_handler = catch_signals;
@@ -296,36 +301,66 @@
 		if (   (sigaddset (&ourset, SIGTERM) != 0)
 		    || (sigaddset (&ourset, SIGALRM) != 0)
 		    || (sigaction (SIGTERM, &action, NULL) != 0)
+		    || (   !doshell /* handle SIGINT (Ctrl-C), SIGQUIT
+		                     * (Ctrl-\), and SIGTSTP (Ctrl-Z)
+		                     * since the child does not control
+		                     * the tty anymore.
+		                     */
+		        && (   (sigaddset (&ourset, SIGINT)  != 0)
+		            || (sigaddset (&ourset, SIGQUIT) != 0)
+		            || (sigaddset (&ourset, SIGTSTP) != 0)
+		            || (sigaction (SIGINT,  &action, NULL) != 0)
+		            || (sigaction (SIGQUIT, &action, NULL) != 0))
+		            || (sigaction (SIGTSTP,  &action, NULL) != 0))
 		    || (sigprocmask (SIG_UNBLOCK, &ourset, NULL) != 0)
 		    ) {
 			fprintf (stderr,
 			         _("%s: signal masking malfunction\n"),
 			         Prog);
-			caught = true;
+			caught = SIGTERM;
 		}
 	}
 
-	if (!caught) {
+	if (0 == caught) {
+		bool stop = true;
+
 		do {
 			pid_t pid;
+			stop = true;
 
 			pid = waitpid (-1, &status, WUNTRACED);
 
-			if (((pid_t)-1 != pid) && (0 != WIFSTOPPED (status))) {
+			/* When interrupted by signal, the signal will be
+			 * forwarded to the child, and termination will be
+			 * forced later.
+			 */
+			if (   ((pid_t)-1 == pid)
+			    && (EINTR == errno)
+			    && (SIGTSTP == caught)) {
+				/* Except for SIGTSTP, which request to
+				 * stop the child.
+				 * We will SIGSTOP ourself on the next
+				 * waitpid round.
+				 */
+				kill (child, SIGSTOP);
+				stop = false;
+			} else if (   ((pid_t)-1 != pid)
+			           && (0 != WIFSTOPPED (status))) {
 				/* The child (shell) was suspended.
 				 * Suspend su. */
 				kill (getpid (), SIGSTOP);
 				/* wake child when resumed */
 				kill (pid, SIGCONT);
+				stop = false;
 			}
-		} while (0 != WIFSTOPPED (status));
+		} while (!stop);
 	}
 
-	if (caught) {
+	if (0 != caught) {
 		(void) fputs ("\n", stderr);
 		(void) fputs (_("Session terminated, terminating shell..."),
 		              stderr);
-		(void) kill (child, SIGTERM);
+		(void) kill (child, caught);
 	}
 
 	ret = pam_close_session (pamh, 0);
@@ -339,7 +374,7 @@
 
 	ret = pam_end (pamh, PAM_SUCCESS);
 
-	if (caught) {
+	if (0 != caught) {
 		(void) signal (SIGALRM, kill_child);
 		(void) alarm (2);
 




More information about the Pkg-shadow-commits mailing list