Fix for CVE-2017-1000367, parsing of /proc/pid/stat on Linux when

millert · millert · commit 817fd283124c · 2017-05-29T14:32:53.000-06:00
the process name contains spaces.  Since the user has control over
the command name this could be used by a user with sudo access to
overwrite an arbitrary file.
Thanks to Qualys for investigating and reporting this bug.

Also stop performing a breadth-first traversal of /dev when looking
for the device.  Only the directories specified in search_devs[]
are checked.

--HG--
branch : 1.8
diff --git a/src/ttyname.c b/src/ttyname.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2012-2016 Todd C. Miller <Todd.Miller@courtesan.com>
+ * Copyright (c) 2012-2017 Todd C. Miller <Todd.Miller@courtesan.com>
  *
  * Permission to use, copy, modify, and distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
@@ -145,37 +145,41 @@ sudo_ttyname_dev(dev_t tdev, char *name, size_t namelen)
 }
 #elif defined(HAVE_STRUCT_PSINFO_PR_TTYDEV) || defined(HAVE_PSTAT_GETPROC) || defined(__linux__)
 /*
- * Devices to search before doing a breadth-first scan.
+ * Device nodes and directories to search before searching all of /dev
  */
 static char *search_devs[] = {
     "/dev/console",
-    "/dev/wscons",
-    "/dev/pts/",
-    "/dev/vt/",
-    "/dev/term/",
-    "/dev/zcons/",
+    "/dev/pts/",	/* POSIX pty */
+    "/dev/vt/",		/* Solaris virtual console */
+    "/dev/term/",	/* Solaris serial ports */
+    "/dev/zcons/",	/* Solaris zone console */
+    "/dev/pty/",	/* HP-UX old-style pty */
     NULL
 };
 
+/*
+ * Device nodes to ignore when searching all of /dev
+ */
 static char *ignore_devs[] = {
-    "/dev/fd/",
     "/dev/stdin",
     "/dev/stdout",
     "/dev/stderr",
     NULL
 };
 
 /*
- * Do a breadth-first scan of dir looking for the specified device.
+ * Do a scan of a directory looking for the specified device.
+ * Does not descend into subdirectories.
  * Returns name on success and NULL on failure, setting errno.
  */
 static char *
-sudo_ttyname_scan(const char *dir, dev_t rdev, bool builtin, char *name, size_t namelen)
+sudo_ttyname_scan(const char *dir, dev_t rdev, char *name, size_t namelen)
 {
-    size_t sdlen, num_subdirs = 0, max_subdirs = 0;
-    char pathbuf[PATH_MAX], **subdirs = NULL;
+    size_t sdlen;
+    char pathbuf[PATH_MAX];
     char *ret = NULL;
     struct dirent *dp;
+    struct stat sb;
     unsigned int i;
     DIR *d = NULL;
     debug_decl(sudo_ttyname_scan, SUDO_DEBUG_UTIL)
@@ -187,6 +191,18 @@ sudo_ttyname_scan(const char *dir, dev_t rdev, bool builtin, char *name, size_t
     if ((d = opendir(dir)) == NULL)
 	goto done;
 
+    if (fstat(dirfd(d), &sb) == -1) {
+	sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO,
+	    "unable to fstat %s", dir);
+	goto done;
+    }
+    if ((sb.st_mode & S_IWOTH) != 0) {
+	sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO,
+	    "ignoring world-writable directory %s", dir);
+	errno = ENOENT;
+	goto done;
+    }
+
     sudo_debug_printf(SUDO_DEBUG_INFO|SUDO_DEBUG_LINENO,
 	"scanning for dev %u in %s", (unsigned int)rdev, dir);
 
@@ -224,18 +240,6 @@ sudo_ttyname_scan(const char *dir, dev_t rdev, bool builtin, char *name, size_t
 	}
 	if (ignore_devs[i] != NULL)
 	    continue;
-	if (!builtin) {
-	    /* Skip entries in search_devs; we already checked them. */
-	    for (i = 0; search_devs[i] != NULL; i++) {
-		len = strlen(search_devs[i]);
-		if (search_devs[i][len - 1] == '/')
-		    len--;
-		if (d_len == len && strncmp(pathbuf, search_devs[i], len) == 0)
-		    break;
-	    }
-	    if (search_devs[i] != NULL)
-		continue;
-	}
 # if defined(HAVE_STRUCT_DIRENT_D_TYPE) && defined(DTTOIF)
 	/*
 	 * Avoid excessive stat() calls by checking dp->d_type.
@@ -248,39 +252,14 @@ sudo_ttyname_scan(const char *dir, dev_t rdev, bool builtin, char *name, size_t
 		if (stat(pathbuf, &sb) == -1)
 		    continue;
 		break;
-	    case DT_DIR:
-		/* Directory, no need to stat() it. */
-		sb.st_mode = DTTOIF(dp->d_type);
-		sb.st_rdev = 0;		/* quiet ccc-analyzer false positive */
-		break;
 	    default:
-		/* Not a character device, link or directory, skip it. */
+		/* Not a character device or link, skip it. */
 		continue;
 	}
 # else
 	if (stat(pathbuf, &sb) == -1)
 	    continue;
 # endif
-	if (S_ISDIR(sb.st_mode)) {
-	    if (!builtin) {
-		/* Add to list of subdirs to search. */
-		if (num_subdirs + 1 > max_subdirs) {
-		    char **new_subdirs;
-
-		    new_subdirs = reallocarray(subdirs, max_subdirs + 64,
-			sizeof(char *));
-		    if (new_subdirs == NULL)
-			goto done;
-		    subdirs = new_subdirs;
-		    max_subdirs += 64;
-		}
-		subdirs[num_subdirs] = strdup(pathbuf);
-		if (subdirs[num_subdirs] == NULL)
-		    goto done;
-		num_subdirs++;
-	    }
-	    continue;
-	}
 	if (S_ISCHR(sb.st_mode) && sb.st_rdev == rdev) {
 	    sudo_debug_printf(SUDO_DEBUG_INFO|SUDO_DEBUG_LINENO,
 		"resolved dev %u as %s", (unsigned int)rdev, pathbuf);
@@ -296,16 +275,9 @@ sudo_ttyname_scan(const char *dir, dev_t rdev, bool builtin, char *name, size_t
 	}
     }
 
-    /* Search subdirs if we didn't find it in the root level. */
-    for (i = 0; ret == NULL && i < num_subdirs; i++)
-	ret = sudo_ttyname_scan(subdirs[i], rdev, false, name, namelen);
-
 done:
     if (d != NULL)
 	closedir(d);
-    for (i = 0; i < num_subdirs; i++)
-	free(subdirs[i]);
-    free(subdirs);
     debug_return_str(ret);
 }
 
@@ -324,7 +296,7 @@ sudo_ttyname_dev(dev_t rdev, char *name, size_t namelen)
     debug_decl(sudo_ttyname_dev, SUDO_DEBUG_UTIL)
 
     /*
-     * First check search_devs for common tty devices.
+     * First check search_devs[] for common tty devices.
      */
     for (sd = search_devs; (devname = *sd) != NULL; sd++) {
 	len = strlen(devname);
@@ -349,7 +321,7 @@ sudo_ttyname_dev(dev_t rdev, char *name, size_t namelen)
 		    "comparing dev %u to %s: no", (unsigned int)rdev, buf);
 	    } else {
 		/* Traverse directory */
-		ret = sudo_ttyname_scan(devname, rdev, true, name, namelen);
+		ret = sudo_ttyname_scan(devname, rdev, name, namelen);
 		if (ret != NULL || errno == ENOMEM)
 		    goto done;
 	    }
@@ -367,9 +339,9 @@ sudo_ttyname_dev(dev_t rdev, char *name, size_t namelen)
     }
 
     /*
-     * Not found?  Do a breadth-first traversal of /dev/.
+     * Not found?  Check all device nodes in /dev.
      */
-    ret = sudo_ttyname_scan(_PATH_DEV, rdev, false, name, namelen);
+    ret = sudo_ttyname_scan(_PATH_DEV, rdev, name, namelen);
 
 done:
     debug_return_str(ret);
@@ -493,28 +465,35 @@ get_process_ttyname(char *name, size_t namelen)
 	len = getline(&line, &linesize, fp);
 	fclose(fp);
 	if (len != -1) {
-	    /* Field 7 is the tty dev (0 if no tty) */
-	    char *cp = line;
-	    char *ep = line;
-	    const char *errstr;
-	    int field = 0;
-	    while (*++ep != '\0') {
-		if (*ep == ' ') {
-		    *ep = '\0';
-		    if (++field == 7) {
-			dev_t tdev = strtonum(cp, INT_MIN, INT_MAX, &errstr);
-			if (errstr) {
-			    sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO,
-				"%s: tty device %s: %s", path, cp, errstr);
-			}
-			if (tdev > 0) {
-			    errno = serrno;
-			    ret = sudo_ttyname_dev(tdev, name, namelen);
-			    goto done;
+	    /*
+	     * Field 7 is the tty dev (0 if no tty).
+	     * Since the process name at field 2 "(comm)" may include spaces,
+	     * start at the last ')' found.
+	     */
+	    char *cp = strrchr(line, ')');
+	    if (cp != NULL) {
+		char *ep = cp;
+		const char *errstr;
+		int field = 1;
+
+		while (*++ep != '\0') {
+		    if (*ep == ' ') {
+			*ep = '\0';
+			if (++field == 7) {
+			    dev_t tdev = strtonum(cp, INT_MIN, INT_MAX, &errstr);
+			    if (errstr) {
+				sudo_debug_printf(SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO,
+				    "%s: tty device %s: %s", path, cp, errstr);
+			    }
+			    if (tdev > 0) {
+				errno = serrno;
+				ret = sudo_ttyname_dev(tdev, name, namelen);
+				goto done;
+			    }
+			    break;
 			}
-			break;
+			cp = ep + 1;
 		    }
-		    cp = ep + 1;
 		}
 	    }
 	}
