[Pkg-shadow-devel] Bug#505071: Bug#505071: closed ... fixed in shadow 1:4.1.3-1

[Paul Szabo]

Dear Nicolas,

> Please state more clearly ...
> If I have to look again ... it is a waste of time.

That discussion is not fruitful.

> If you have ... patches, they are welcomed.

Please see below. The patch of src/login.c is essential for security;
I would prefer to use the libmisc/utmp.c patch also.

Hmm... am now thinking that hostname (PAM_RHOST) may also be dodgy.

>> Please re-open the bug.
> Please do ...

I do not think I can re-open (would not know how, and I think am banned
from doing control things since the kerfuffle in #299007).

Cheers, Paul

Paul Szabo   psz at maths.usyd.edu.au   http://www.maths.usyd.edu.au/u/psz/
School of Mathematics and Statistics   University of Sydney    Australia


--- src/login.c.bak	2009-04-17 07:00:50.000000000 +1000
+++ src/login.c	2009-04-17 07:30:51.000000000 +1000
@@ -479,7 +479,14 @@
 	 * entry (will not overwrite remote hostname).  --marekm
 	 */
 	checkutmp (!amroot);
-	STRFCPY (tty, utent.ut_line);
+	/*
+	 * PSz 17 Apr 09 Though we may handle ut_line correctly (for Linux),
+	 * we should not trust PAM_TTY to its vagaries...
+	 *STRFCPY (tty, utent.ut_line);
+	 */
+	tmp = ttyname (0);
+	if (NULL == tmp) { tmp = "UNKNOWN"; }
+	STRFCPY (tty, tmp);
 #ifndef USE_PAM
 	is_console = console (tty);
 #endif
--- libmisc/utmp.c.bak	2008-11-23 10:56:10.000000000 +1100
+++ libmisc/utmp.c	2009-04-17 07:53:08.000000000 +1000
@@ -127,23 +127,43 @@
 			(void) puts (NO_UTENT);
 			exit (EXIT_FAILURE);
 		}
-		line = ttyname (0);
-		if (NULL == line) {
-			(void) puts (NO_TTY);
-			exit (EXIT_FAILURE);
-		}
-		if (strncmp (line, "/dev/", 5) == 0) {
-			line += 5;
-		}
+/*
+ *		line = ttyname (0);
+ *		if (NULL == line) {
+ *			(void) puts (NO_TTY);
+ *			exit (EXIT_FAILURE);
+ *		}
+ *		if (strncmp (line, "/dev/", 5) == 0) {
+ *			line += 5;
+ *		}
+ */
 		memset ((void *) &utent, 0, sizeof utent);
 		utent.ut_type = LOGIN_PROCESS;
 		utent.ut_pid = pid;
-		strncpy (utent.ut_line, line, sizeof utent.ut_line);
-		/* XXX - assumes /dev/tty?? or /dev/pts/?? */
-		strncpy (utent.ut_id, utent.ut_line + 3, sizeof utent.ut_id);
+/*
+ *		strncpy (utent.ut_line, line, sizeof utent.ut_line);
+ *		* XXX - assumes /dev/tty?? or /dev/pts/?? *
+ *		strncpy (utent.ut_id, utent.ut_line + 3, sizeof utent.ut_id);
+ */
 		strcpy (utent.ut_user, "LOGIN");
 		utent.ut_time = time (NULL);
 	}
+	/*
+	 * PSz 17 Apr 09 Sanitize ut_line and ut_id anyway... so why
+	 * did we bother with getutent and is_my_tty: for ut_host that
+	 * we cannot trust either?
+	 */
+	line = ttyname (0);
+	if (NULL == line) {
+		(void) puts (NO_TTY);
+		exit (EXIT_FAILURE);
+	}
+	if (strncmp (line, "/dev/", 5) == 0) {
+		line += 5;
+	}
+	strncpy (utent.ut_line, line, sizeof utent.ut_line);
+	/* XXX - assumes /dev/tty?? or /dev/pts/?? */
+	strncpy (utent.ut_id, utent.ut_line + 3, sizeof utent.ut_id);
 }
 
 #elif defined(LOGIN_PROCESS)