[Pkg-shadow-devel] Bug#628843: login: tty hijacking - suggested solution inclusive patch

Wolfgang Zarre lkdev at essax.com
Fri Mar 29 22:54:21 UTC 2013


Hello,

IMHO it is not the right approach in just simply dropping the
controlling terminal independent in using the parameter -c or not
because You cannot have an interactive shell without a
controlling terminal.

Sure, if you call anything else but a shell then it is IMHO up to
the application to drop the controlling terminal.

I think that in any case the right solution is in just flushing
the input queue before returning to the caller which would not
just protect in case of hijacking but also of buggy applications.

The following patch was just a quick fix of the actual version
in Wheezy 4.1.5.1 which worked for me and was tested by myself
with USE_PAM but also without PAM but in the later case just
from user 'root' to another user in using the above mentioned
exploit.

Maybe further tests are needed also in respect of the modified
signals.

The following patch as suggested solution:

___BEGIN_PATCH___
--- shadow-4.1.5.1.orig/src/su.c	2012-05-25 13:51:55.000000000 +0200
+++ shadow-4.1.5.1/src/su.c	2013-03-29 21:52:11.930790904 +0100
@@ -62,8 +62,8 @@
 #include <stdio.h>
 #include <sys/types.h>
 #include <unistd.h>
-#ifndef USE_PAM
 #include <sys/ioctl.h>
+#ifndef USE_PAM
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <fcntl.h>
@@ -106,10 +106,10 @@ static bool change_environment = true;

 #ifdef USE_PAM
 static pam_handle_t *pamh = NULL;
+#endif
 static int caught = 0;
 /* PID of the child, in case it needs to be killed */
 static pid_t pid_child = 0;
-#endif

 /*
  * External identifiers
@@ -123,10 +123,9 @@ extern size_t newenvc; /* libmisc/env.c
 static void execve_shell (const char *shellname,
                           char *args[],
                           char *const envp[]);
-#ifdef USE_PAM
 static RETSIGTYPE kill_child (int unused(s));
-static void prepare_pam_close_session (void);
-#else				/* !USE_PAM */
+static void prepare_close_session (void);
+#ifndef USE_PAM
 static RETSIGTYPE die (int);
 static bool iswheel (const char *);
 #endif				/* !USE_PAM */
@@ -177,7 +176,8 @@ static bool iswheel (const char *usernam
 	}
 	return is_on_list (grp->gr_mem, username);
 }
-#else				/* USE_PAM */
+#endif				/* USE_PAM */
+
 static RETSIGTYPE kill_child (int unused(s))
 {
 	if (0 != pid_child) {
@@ -189,7 +189,6 @@ static RETSIGTYPE kill_child (int unused
 	}
 	exit (255);
 }
-#endif				/* USE_PAM */

 /* borrowed from GNU sh-utils' "su.c" */
 static bool restricted_shell (const char *shellname)
@@ -260,7 +259,6 @@ static void execve_shell (const char *sh
 	}
 }

-#ifdef USE_PAM
 /* Signal handler for parent process later */
 static void catch_signals (int sig)
 {
@@ -268,12 +266,12 @@ static void catch_signals (int sig)
 }

 /*
- * prepare_pam_close_session - Fork and wait for the child to close the session
+ * prepare_close_session - Fork and wait for the child to close the session
  *
  *	Only the child returns. The parent will wait for the child to
  *	terminate and exit.
  */
-static void prepare_pam_close_session (void)
+static void prepare_close_session (void)
 {
 	sigset_t ourset;
 	int status;
@@ -311,17 +309,6 @@ static void prepare_pam_close_session (v
 		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 will not control
-		                     * the tty.
-		                     */
-		        && (   (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,
@@ -366,6 +353,12 @@ static void prepare_pam_close_session (v
 		} while (!stop);
 	}

+	/* This avoids the callee to inject commands on
+	 * the caller's tty including srappy incidents. */
+	ret = ioctl( STDIN_FILENO, TCFLSH, TCIFLUSH);
+	if( ret == -1)
+		fprintf (stderr, _("%s: Flushing input: %s\n"), Prog, strerror( errno));
+
 	if (0 != caught) {
 		(void) fputs ("\n", stderr);
 		(void) fputs (_("Session terminated, terminating shell..."),
@@ -373,6 +366,7 @@ static void prepare_pam_close_session (v
 		(void) kill (pid_child, caught);
 	}

+#ifdef USE_PAM
 	ret = pam_close_session (pamh, 0);
 	if (PAM_SUCCESS != ret) {
 		SYSLOG ((LOG_ERR, "pam_close_session: %s",
@@ -382,6 +376,7 @@ static void prepare_pam_close_session (v

 	(void) pam_setcred (pamh, PAM_DELETE_CRED);
 	(void) pam_end (pamh, PAM_SUCCESS);
+#endif				/* USE_PAM */

 	if (0 != caught) {
 		(void) signal (SIGALRM, kill_child);
@@ -395,7 +390,6 @@ static void prepare_pam_close_session (v
 	                                : WTERMSIG (status) + 128);
 	/* Only the child returns. See above. */
 }
-#endif				/* USE_PAM */

 /*
  * usage - print command line syntax and exit
@@ -1057,13 +1051,16 @@ int main (int argc, char **argv)
 		exit (1);
 	}

-	prepare_pam_close_session ();
+	prepare_close_session ();

 	/* become the new user */
 	if (change_uid (pw) != 0) {
 		exit (1);
 	}
 #else				/* !USE_PAM */
+
+	prepare_close_session ();
+
 	/* no limits if su from root (unless su must fake login's behavior) */
 	if (!caller_is_root || fakelogin) {
 		setup_limits (pw);
@@ -1076,36 +1073,6 @@ int main (int argc, char **argv)

 	set_environment (pw);

-	if (!doshell) {
-		/* There is no need for a controlling terminal.
-		 * This avoids the callee to inject commands on
-		 * the caller's tty. */
-		int err = -1;
-
-#ifdef USE_PAM
-		/* When PAM is used, we are on the child */
-		err = setsid ();
-#else
-		/* Otherwise, we cannot use setsid */
-		int fd = open ("/dev/tty", O_RDWR);
-
-		if (fd >= 0) {
-			err = ioctl (fd, TIOCNOTTY, (char *) 0);
-			(void) close (fd);
-		} else if (ENXIO == errno) {
-			/* There are no controlling terminal already */
-			err = 0;
-		}
-#endif				/* USE_PAM */
-
-		if (-1 == err) {
-			(void) fprintf (stderr,
-			                _("%s: Cannot drop the controlling terminal\n"),
-			                Prog);
-			exit (1);
-		}
-	}
-
 	/*
 	 * PAM_DATA_SILENT is not supported by some modules, and
 	 * there is no strong need to clean up the process space's
___END_PATCH___




with best regards

Wolf



More information about the Pkg-shadow-devel mailing list