* NEWS, libmisc/chowntty.c, libmisc/utmp.c: is_my_tty() moved from

utmp.c to chowntty.c. checkutmp() now only uses an existing utmp
	entry if the pid matches and ut_line matches with the current tty.
	This fixes a possible DOS when entries can be forged in the utmp
	file.
	* libmisc/chowntty.c, src/login.c, lib/prototypes.h: Remove the
	tty argument from chown_tty. chown_tty always changes stdin and
	does not need this argument anymore.

ChangeLog

@@ -2,6 +2,14 @@
 
 	* NEWS, libmisc/chowntty.c: Fix a race condition that could lead to
 	gaining ownership or changing mode of arbitrary files.
+	* NEWS, libmisc/chowntty.c, libmisc/utmp.c: is_my_tty() moved from
+	utmp.c to chowntty.c. checkutmp() now only uses an existing utmp
+	entry if the pid matches and ut_line matches with the current tty.
+	This fixes a possible DOS when entries can be forged in the utmp
+	file.
+	* libmisc/chowntty.c, src/login.c, lib/prototypes.h: Remove the
+	tty argument from chown_tty. chown_tty always changes stdin and
+	does not need this argument anymore.
 
 2008-10-11  Nicolas François  <nicolas.francois@centraliens.net>
 

NEWS

@@ -64,6 +64,8 @@ shadow-4.1.2.1 -> shadow-4.1.2.2					23-11-2008
 *** security
 - Fix a race condition in login that could lead to gaining ownership or
   changing mode of arbitrary files.
+- Fix a possible login DOS, which could be caused by injecting forged
+  entries in utmp.
 
 shadow-4.1.2 -> shadow-4.1.2.1						26-06-2008
 

libmisc/chowntty.c

@@ -43,32 +43,14 @@
 #include "defines.h"
 #include <pwd.h>
 #include "getdef.h"
-/*
- * is_my_tty -- determine if "tty" is the same as TTY stdin is using
- */
-static bool is_my_tty (const char *tty)
-{
-	struct stat by_name, by_fd;
-
-	if ((stat (tty, &by_name) != 0) || (fstat (0, &by_fd) != 0)) {
-		return false;
-	}
-
-	if (by_name.st_rdev != by_fd.st_rdev) {
-		return false;
-	} else {
-		return true;
-	}
-}
 
 /*
  *	chown_tty() sets the login tty to be owned by the new user ID
  *	with TTYPERM modes
  */
 
-void chown_tty (const char *tty, const struct passwd *info)
+void chown_tty (const struct passwd *info)
 {
-	char buf[200], full_tty[200];
 	char *group;		/* TTY group name or number */
 	struct group *grent;
 	gid_t gid;
@@ -97,18 +79,6 @@ void chown_tty (const char *tty, const struct passwd *info)
 	 * the group as determined above.
 	 */
 
-	if ('/' != *tty) {
-		snprintf (full_tty, sizeof full_tty, "/dev/%s", tty);
-		tty = full_tty;
-	}
-
-	if (!is_my_tty (tty)) {
-		SYSLOG ((LOG_WARN,
-			 "unable to determine TTY name, got %s\n", tty));
-		closelog ();
-		exit (1);
-	}
-
 	if (   (fchown (STDIN_FILENO, info->pw_uid, gid) != 0)
 	    || (fchmod (STDIN_FILENO, getdef_num ("TTYPERM", 0600)) != 0)) {
 		int err = errno;

libmisc/utmp.c

@@ -56,6 +56,31 @@ struct utmp utent;
 #define	NO_TTY \
 	_("Unable to determine your tty name.")
 
+/*
+ * is_my_tty -- determine if "tty" is the same TTY stdin is using
+ */
+static bool is_my_tty (const char *tty)
+{
+	char full_tty[200];
+	struct stat by_name, by_fd;
+
+	if ('/' != *tty) {
+		snprintf (full_tty, sizeof full_tty, "/dev/%s", tty);
+		tty = full_tty;
+	}
+
+	if (   (stat (tty, &by_name) != 0)
+	    || (fstat (STDIN_FILENO, &by_fd) != 0)) {
+		return false;
+	}
+
+	if (by_name.st_rdev != by_fd.st_rdev) {
+		return false;
+	} else {
+		return true;
+	}
+}
+
 /*
  * checkutmp - see if utmp file is correct for this process
  *
@@ -81,16 +106,20 @@ void checkutmp (bool picky)
 	setutent ();
 
 	/* First, try to find a valid utmp entry for this process.  */
-	while ((ut = getutent ()) != NULL)
+	while ((ut = getutent ()) != NULL) {
 		if (   (ut->ut_pid == pid)
 		    && ('\0' != ut->ut_line[0])
 		    && ('\0' != ut->ut_id[0])
 		    && (   (LOGIN_PROCESS == ut->ut_type)
-		        || (USER_PROCESS  == ut->ut_type))) {
+		        || (USER_PROCESS  == ut->ut_type))
+		    /* A process may have failed to close an entry
+		     * Check if this entry refers to this tty */
+		    && is_my_tty (ut->ut_line)) {
 			break;
 		}
+	}
 
-	/* If there is one, just use it, otherwise create a new one.  */
+	/* If there is one, just use it, otherwise create a new one. */
 	if (NULL != ut) {
 		utent = *ut;
 	} else {
@@ -110,7 +139,7 @@ void checkutmp (bool picky)
 		utent.ut_type = LOGIN_PROCESS;
 		utent.ut_pid = pid;
 		strncpy (utent.ut_line, line, sizeof utent.ut_line);
-		/* XXX - assumes /dev/tty?? */
+		/* 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);
@@ -173,6 +202,10 @@ void checkutmp (bool picky)
 		 * the structure.  The UNIX/PC is broken in this regard
 		 * and needs help ...
 		 */
+		/* XXX: The ut_line may not match with the current tty.
+		 *      ut_line will be set by setutmp anyway, but ut_id
+		 *      will not be set, and the wrong utmp entry may be
+		 *      updated. -- nekral */
 
 		if (utent.ut_line[0] == '\0')
 #endif				/* !UNIXPC */

src/login.c

@@ -1094,7 +1094,7 @@ int main (int argc, char **argv)
 	}
 	setup_limits (&pwent);	/* nice, ulimit etc. */
 #endif				/* ! USE_PAM */
-	chown_tty (tty, &pwent);
+	chown_tty (&pwent);
 
 #ifdef USE_PAM
 	/*