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

[Paul Szabo]

Dear Nicolas,

Had a quick look at src/login.c and libmisc/utmp.c, and see the "plain"
(not security) issues below. - The enspent() block should be moved up
in newgrp.c also.

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


--- login.c.bak	2009-04-22 09:34:08.000000000 +1000
+++ login.c	2009-04-22 10:32:53.000000000 +1000
@@ -533,6 +533,7 @@
 		(void) puts (_("No utmp entry.  You must exec \"login\" from the lowest level \"sh\""));
 		exit (1);
 	}
+	/* Take note that utent may be NULL after this... */
 
 	tmptty = ttyname (0);
 	if (NULL == tmptty) {
@@ -625,15 +626,13 @@
 
 	if (rflg || hflg) {
 		cp = hostname;
-	} else {
 #ifdef	HAVE_STRUCT_UTMP_UT_HOST
-		if ('\0' != utent->ut_host[0]) {
-			cp = utent->ut_host;
-		} else
+/* Take care with utent that may be NULL */
+	} else if (utent != NULL && '\0' != utent->ut_host[0]) {
+		cp = utent->ut_host;
 #endif				/* HAVE_STRUCT_UTMP_UT_HOST */
-		{
-			cp = "";
-		}
+	} else {
+		cp = "";
 	}
 
 	if ('\0' != *cp) {
--- utmp.c.bak	2009-04-22 09:34:34.000000000 +1000
+++ utmp.c	2009-04-22 10:32:55.000000000 +1000
@@ -93,6 +93,7 @@
  *	Return NULL if no entries exist in utmp for the current process.
  */
 struct utmp *get_current_utmp (void)
+/* May return NULL. Should it return an "empty" (zeroed) something instead? */
 {
 	struct utmp *ut;
 	struct utmp *ret = NULL;
@@ -109,6 +110,13 @@
 		    && (   (LOGIN_PROCESS == ut->ut_type)
 		        || (USER_PROCESS  == ut->ut_type))
 #endif
+/*
+ * We use HAVE_STRUCT_UTMP_UT_ID above (surely utmp has that??!!)
+ * Should we use
+ *   HAVE_STRUCT_UTMP_UT_LINE below, or use
+ *   HAVE_STRUCT_UTMP_UT_PID above
+ * (are defined anywhere?)
+ */
 		    /* A process may have failed to close an entry
 		     * Check if this entry refers to the current tty */
 		    && is_my_tty (ut->ut_line)) {
@@ -117,6 +125,7 @@
 	}
 
 	if (NULL != ut) {
+		/* malloc, or xmalloc as everywhere else? */
 		ret = malloc (sizeof (*ret));
 		memcpy (ret, ut, sizeof (*ret));
 	}
@@ -200,6 +209,7 @@
 
 	assert (NULL != name);
 	assert (NULL != line);
+	/* BUG: utent in login.c may be NULL (in prepare_utmpx also) */
 	assert (NULL != ut);
 
 
@@ -234,6 +244,7 @@
 	strncpy (utent->ut_line, line,      sizeof (utent->ut_line));
 #ifdef HAVE_STRUCT_UTMP_UT_ID
 	strncpy (utent->ut_id,   ut->ut_id, sizeof (utent->ut_id));
+	/* BUG: set "correct" ut_id based on "line", in case we did not get any (in prepare_utmpx also) */
 #endif				/* HAVE_STRUCT_UTMP_UT_ID */
 #ifdef HAVE_STRUCT_UTMP_UT_NAME
 	strncpy (utent->ut_name, name,      sizeof (utent->ut_name));