Track the destination sudoers path for each parsed file.

When adminconfdir is enabled, the destination pathh may be different
from the path we opened.  We always store an edited file in the
adminconfdir (if enabled).  This makes it possible to use visudo
when /etc/sudoers is located on a read-only file system.

1 files changed, 74 insertions, 54 deletions

diff --git a/plugins/sudoers/visudo.c b/plugins/sudoers/visudo.c
index e738e123b..05af2c8cd 100644
--- a/plugins/sudoers/visudo.c
+++ b/plugins/sudoers/visudo.c
@@ -72,12 +72,13 @@
 
 struct sudoersfile {
     TAILQ_ENTRY(sudoersfile) entries;
-    char *path;
-    char *tpath;
-    bool modified;
-    bool doedit;
-    int fd;
-    int errorline;
+    char *opath;	/* original path we opened */
+    char *dpath;	/* destination path to write to */
+    char *tpath;	/* editor temporary file path */
+    bool modified;	/* true if the user modified the file */
+    bool doedit;	/* true when editing (not just checking) sudoers */
+    int fd;		/* fd of the original file (if it exists) */
+    int errorline;	/* line number when there is a syntax error */
 };
 TAILQ_HEAD(sudoersfile_list, sudoersfile);
 
@@ -305,7 +306,7 @@ main(int argc, char *argv[])
 	if (!sp->doedit)
 	    continue;
 	if (sp != TAILQ_FIRST(&sudoerslist)) {
-	    printf(_("press return to edit %s: "), sp->path);
+	    printf(_("press return to edit %s: "), sp->opath);
 	    while ((ch = getchar()) != EOF && ch != '\n')
 		    continue;
 	}
@@ -347,7 +348,7 @@ visudo_track_error(const char *file, int line, int column, const char *fmt,
 	if (sp->errorline > 0)
 	    continue;		/* preserve the first error */
 
-	if (strcmp(file, sp->path) == 0 ||
+	if (strcmp(file, sp->opath) == 0 ||
 		(sp->tpath != NULL && strcmp(file, sp->tpath) == 0)) {
 	    sp->errorline = line;
 	    break;
@@ -489,19 +490,19 @@ edit_sudoers(struct sudoersfile *sp, char *editor, int editor_argc,
     debug_decl(edit_sudoers, SUDOERS_DEBUG_UTIL);
 
     if (fstat(sp->fd, &sb) == -1)
-	sudo_fatal(U_("unable to stat %s"), sp->path);
+	sudo_fatal(U_("unable to stat %s"), sp->opath);
     orig_size = sb.st_size;
     mtim_get(&sb, orig_mtim);
 
     /* Create the temp file if needed and set timestamp. */
     if (sp->tpath == NULL) {
-	if (asprintf(&sp->tpath, "%s.tmp", sp->path) == -1)
+	if (asprintf(&sp->tpath, "%s.tmp", sp->dpath) == -1)
 	    sudo_fatalx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
 	tfd = open(sp->tpath, O_WRONLY|O_CREAT|O_TRUNC, S_IRWXU|S_IRUSR);
 	if (tfd < 0)
 	    sudo_fatal("%s", sp->tpath);
 
-	/* Copy sp->path -> sp->tpath and reset the mtime. */
+	/* Copy sp->opath -> sp->tpath and reset the mtime. */
 	if (orig_size != 0) {
 	    char buf[4096], lastch = '\0';
 	    ssize_t nread;
@@ -564,19 +565,19 @@ edit_sudoers(struct sudoersfile *sp, char *editor, int editor_argc,
 	 */
 	if (stat(sp->tpath, &sb) == -1) {
 	    sudo_warnx(U_("unable to stat temporary file (%s), %s unchanged"),
-		sp->tpath, sp->path);
+		sp->tpath, sp->opath);
 	    goto done;
 	}
 	if (sb.st_size == 0 && orig_size != 0) {
 	    /* Avoid accidental zeroing of main sudoers file. */
 	    if (sp == TAILQ_FIRST(&sudoerslist)) {
 		sudo_warnx(U_("zero length temporary file (%s), %s unchanged"),
-		    sp->tpath, sp->path);
+		    sp->tpath, sp->opath);
 		goto done;
 	    }
 	}
     } else {
-	sudo_warnx(U_("editor (%s) failed, %s unchanged"), editor, sp->path);
+	sudo_warnx(U_("editor (%s) failed, %s unchanged"), editor, sp->opath);
 	goto done;
     }
 
@@ -644,12 +645,12 @@ reparse_sudoers(char *editor, int editor_argc, char **editor_argv,
 	fp = fopen(sp->tpath, "r+");
 	if (fp == NULL)
 	    sudo_fatalx(U_("unable to re-open temporary file (%s), %s unchanged."),
-		sp->tpath, sp->path);
+		sp->tpath, sp->opath);
 
 	/* Clean slate for each parse */
 	if (!init_defaults())
 	    sudo_fatalx("%s", U_("unable to initialize sudoers default values"));
-	init_parser_ext(sp->path, true, quiet ? 0 : 2);
+	init_parser_ext(sp->opath, true, quiet ? 0 : 2);
 	sp->errorline = -1;
 
 	/* Parse the sudoers temp file(s) */
@@ -696,7 +697,7 @@ reparse_sudoers(char *editor, int editor_argc, char **editor_argv,
 	if ((sp = TAILQ_NEXT(last, entries)) != NULL) {
 	    bool modified = false;
 	    do {
-		printf(_("press return to edit %s: "), sp->path);
+		printf(_("press return to edit %s: "), sp->opath);
 		while ((ch = getchar()) != EOF && ch != '\n')
 			continue;
 		edit_sudoers(sp, editor, editor_argc, editor_argv, -1);
@@ -741,18 +742,18 @@ install_sudoers(struct sudoersfile *sp, bool set_owner, bool set_mode)
 	if (fstat(sp->fd, &sb) == 0) {
 	    if (set_owner) {
 		if (sb.st_uid != sudoers_uid || sb.st_gid != sudoers_gid) {
-		    if (chown(sp->path, sudoers_uid, sudoers_gid) != 0) {
+		    if (chown(sp->opath, sudoers_uid, sudoers_gid) != 0) {
 			sudo_warn(U_("unable to set (uid, gid) of %s to (%u, %u)"),
-			    sp->path, (unsigned int)sudoers_uid,
+			    sp->opath, (unsigned int)sudoers_uid,
 			    (unsigned int)sudoers_gid);
 		    }
 		}
 	    }
 	    if (set_mode) {
 		if ((sb.st_mode & ACCESSPERMS) != sudoers_mode) {
-		    if (chmod(sp->path, sudoers_mode) != 0) {
+		    if (chmod(sp->opath, sudoers_mode) != 0) {
 			sudo_warn(U_("unable to change mode of %s to 0%o"),
-			    sp->path, (unsigned int)sudoers_mode);
+			    sp->opath, (unsigned int)sudoers_mode);
 		    }
 		}
 	    }
@@ -762,13 +763,13 @@ install_sudoers(struct sudoersfile *sp, bool set_owner, bool set_mode)
     }
 
     /*
-     * Change mode and ownership of temp file so when
-     * we move it to sp->path things are kosher.
+     * Change mode and ownership of temp file before moving it into place
+     * to avoid a race condition.
      */
     if (!set_owner || !set_mode) {
 	/* Preserve owner/perms of the existing file.  */
 	if (fstat(sp->fd, &sb) == -1)
-	    sudo_fatal(U_("unable to stat %s"), sp->path);
+	    sudo_fatal(U_("unable to stat %s"), sp->opath);
     }
     if (set_owner) {
 	if (chown(sp->tpath, sudoers_uid, sudoers_gid) != 0) {
@@ -798,34 +799,35 @@ install_sudoers(struct sudoersfile *sp, bool set_owner, bool set_mode)
 
     /*
      * Now that we know sp->tpath parses correctly, it needs to be
-     * rename(2)'d to sp->path.  If the rename(2) fails we try using
-     * mv(1) in case sp->tpath and sp->path are on different file systems.
+     * rename(2)'d to sp->dpath.  If the rename(2) fails we try using
+     * mv(1) in case sp->tpath and sp->dpath are on different file systems.
      */
-    if (rename(sp->tpath, sp->path) == 0) {
+    if (rename(sp->tpath, sp->dpath) == 0) {
 	free(sp->tpath);
 	sp->tpath = NULL;
     } else {
 	if (errno == EXDEV) {
 	    char *av[4];
 	    sudo_warnx(U_("%s and %s not on the same file system, using mv to rename"),
-	      sp->tpath, sp->path);
+	      sp->tpath, sp->dpath);
 
 	    /* Build up argument vector for the command */
 	    av[0] = sudo_basename(_PATH_MV);
 	    av[1] = sp->tpath;
-	    av[2] = sp->path;
+	    av[2] = sp->dpath;
 	    av[3] = NULL;
 
 	    /* And run it... */
 	    if (run_command(_PATH_MV, av, false) != 0) {
 		sudo_warnx(U_("command failed: '%s %s %s', %s unchanged"),
-		    _PATH_MV, sp->tpath, sp->path, sp->path);
+		    _PATH_MV, sp->tpath, sp->dpath, sp->opath);
 		goto done;
 	    }
 	    free(sp->tpath);
 	    sp->tpath = NULL;
 	} else {
-	    sudo_warn(U_("error renaming %s, %s unchanged"), sp->tpath, sp->path);
+	    sudo_warn(U_("error renaming %s, %s unchanged"), sp->tpath,
+		sp->opath);
 	    goto done;
 	}
     }
@@ -1091,9 +1093,9 @@ check_syntax(const char *path, bool quiet, bool strict, bool check_owner,
 	    ok = false;
 	}
 	TAILQ_FOREACH(sp, &sudoerslist, entries) {
-	    if (check_file(sp->path, quiet, check_owner, check_mode)) {
+	    if (check_file(sp->opath, quiet, check_owner, check_mode)) {
 		if (!quiet)
-		    (void) printf(_("%s: parsed OK\n"), sp->path);
+		    (void) printf(_("%s: parsed OK\n"), sp->opath);
 	    } else {
 		ok = false;
 	    }
@@ -1112,10 +1114,10 @@ lock_sudoers(struct sudoersfile *entry)
 
     if (!sudo_lock_file(entry->fd, SUDO_TLOCK)) {
 	if (errno == EAGAIN || errno == EWOULDBLOCK) {
-	    sudo_warnx(U_("%s busy, try again later"), entry->path);
+	    sudo_warnx(U_("%s busy, try again later"), entry->opath);
 	    debug_return_bool(false);
 	}
-	sudo_warn(U_("unable to lock %s"), entry->path);
+	sudo_warn(U_("unable to lock %s"), entry->opath);
 	(void) fputs(_("Edit anyway? [y/N]"), stdout);
 	ch = getchar();
 	if (tolower(ch) != 'y')
@@ -1138,6 +1140,12 @@ new_sudoers(const char *path, bool doedit)
     int fd = -1;
     debug_decl(new_sudoersfile, SUDOERS_DEBUG_UTIL);
 
+    /* We always write to the first file in the colon-separated path. */
+    len = strcspn(path, ":");
+    entry = calloc(1, sizeof(*entry));
+    if (entry == NULL || (entry->dpath = strndup(path, len)) == NULL)
+	sudo_fatalx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
+
     /* Open the first file found in the colon-separated path. */
     path_end = path + strlen(path);
     for (cp = sudo_strsplit(path, path_end, ":", &ep);
@@ -1154,27 +1162,38 @@ new_sudoers(const char *path, bool doedit)
 
 	/* Open in write mode for file locking. */
 	fd = open(fname, checkonly ? O_RDONLY : O_RDWR);
-	if (fd != -1 || errno != ENOENT)
+	if (fd != -1) {
+	    /* Store the path we actually opened. */
+	    if ((entry->opath = strdup(fname)) == NULL) {
+		sudo_fatalx(U_("%s: %s"), __func__,
+		    U_("unable to allocate memory"));
+	    }
 	    break;
-    }
+	}
 
-    /* If more than one file is specified, we always write to the first one. */
-    len = strcspn(path, ":");
-    entry = calloc(1, sizeof(*entry));
-    if (entry == NULL || (entry->path = strndup(path, len)) == NULL)
-	sudo_fatalx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
-    if (fd == -1 && errno == ENOENT) {
+	/* If the file exists but we can't open it, that is a fatal error. */
+	if (errno != ENOENT) {
+	    sudo_warn("%s", fname);
+	    goto bad;
+	}
+    }
+    if (fd == -1) {
 	if (!checkonly) {
-	    /* Create the first file in the path. */
-	    fd = open(entry->path, O_RDWR|O_CREAT, sudoers_mode);
+	    /* No sudoers file, create the destination file for editing. */
+	    fd = open(entry->dpath, O_RDWR|O_CREAT, sudoers_mode);
+	}
+	if (fd == -1) {
+	    sudo_warn("%s", entry->dpath);
+	    goto bad;
 	}
+	entry->opath = entry->dpath;
     }
-    if (fd == -1 || fstat(fd, &sb) == -1) {
-	sudo_warn("%s", entry->path);
+    if (fstat(fd, &sb) == -1) {
+	sudo_warn("%s", entry->opath);
 	goto bad;
     }
     if (!S_ISREG(sb.st_mode)) {
-	sudo_warnx(U_("%s is not a regular file"), entry->path);
+	sudo_warnx(U_("%s is not a regular file"), entry->opath);
 	goto bad;
     }
     entry->fd = fd;
@@ -1187,7 +1206,9 @@ new_sudoers(const char *path, bool doedit)
 bad:
     if (fd != -1)
 	close(fd);
-    free(entry->path);
+    if (entry->opath != entry->dpath)
+	free(entry->opath);
+    free(entry->dpath);
     free(entry);
     debug_return_ptr(NULL);
 }
@@ -1207,7 +1228,7 @@ open_sudoers(const char *path, char **outfile, bool doedit, bool *keepopen)
     /* Check for existing entry using the first file in path. */
     len = strcspn(path, ":");
     TAILQ_FOREACH(entry, &sudoerslist, entries) {
-	if (strncmp(path, entry->path, len) == 0 && entry->path[len] == '\0')
+	if (strncmp(path, entry->opath, len) == 0 && entry->opath[len] == '\0')
 	    break;
     }
     if (entry == NULL) {
@@ -1220,7 +1241,7 @@ open_sudoers(const char *path, char **outfile, bool doedit, bool *keepopen)
 	if ((entry = new_sudoers(path, doedit)) == NULL)
 	    debug_return_ptr(NULL);
 	if ((fp = fdopen(entry->fd, "r")) == NULL)
-	    sudo_fatal("%s", entry->path);
+	    sudo_fatal("%s", entry->opath);
 	TAILQ_INSERT_TAIL(&sudoerslist, entry, entries);
     } else {
 	/* Already exists, open .tmp version if there is one. */
@@ -1229,15 +1250,14 @@ open_sudoers(const char *path, char **outfile, bool doedit, bool *keepopen)
 		sudo_fatal("%s", entry->tpath);
 	} else {
 	    if ((fp = fdopen(entry->fd, "r")) == NULL)
-		sudo_fatal("%s", entry->path);
+		sudo_fatal("%s", entry->opath);
 	    rewind(fp);
 	}
     }
     if (keepopen != NULL)
 	*keepopen = true;
     if (outfile != NULL) {
-	/* XXX - if path is a list, entry->path may not exist yet. */
-	*outfile = sudo_rcstr_dup(entry->path);
+	*outfile = sudo_rcstr_dup(entry->opath);
 	if (*outfile == NULL)
 	    sudo_fatalx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
     }