From a147330f3fd7afa8bcb1679c43b623721c39a5fc Mon Sep 17 00:00:00 2001 From: "Todd C. Miller" Date: Thu, 21 Aug 2014 15:28:35 -0600 Subject: [PATCH] Refactor code that copies temp files into separate functions. --- src/sudo_edit.c | 363 ++++++++++++++++++++++++++++++------------------ 1 file changed, 224 insertions(+), 139 deletions(-) diff --git a/src/sudo_edit.c b/src/sudo_edit.c index d46d16ae7..5d4d59328 100644 --- a/src/sudo_edit.c +++ b/src/sudo_edit.c @@ -53,6 +53,52 @@ #if defined(HAVE_SETRESUID) || defined(HAVE_SETREUID) || defined(HAVE_SETEUID) +/* + * Editor temporary file name along with original name, mtime and size. + */ +struct tempfile { + char *tfile; + char *ofile; + struct timeval omtim; + off_t osize; +}; + +static char edit_tmpdir[MAX(sizeof(_PATH_VARTMP), sizeof(_PATH_TMP))]; + +/* + * Find our temporary directory, one of /var/tmp, /usr/tmp, or /tmp + * Returns true on success, else false; + */ +static bool +set_tmpdir(void) +{ + const char *tdir; + struct stat sb; + size_t len; + debug_decl(set_tmpdir, SUDO_DEBUG_EDIT) + + if (stat(_PATH_VARTMP, &sb) == 0 && S_ISDIR(sb.st_mode)) { + tdir = _PATH_VARTMP; + } +#ifdef _PATH_USRTMP + else if (stat(_PATH_USRTMP, &sb) == 0 && S_ISDIR(sb.st_mode)) { + tdir = _PATH_USRTMP; + } +#endif + else { + tdir = _PATH_TMP; + } + len = strlcpy(edit_tmpdir, tdir, sizeof(edit_tmpdir)); + if (len >= sizeof(edit_tmpdir)) { + errno = ENAMETOOLONG; + sudo_warn("%s", tdir); + debug_return_bool(false); + } + while (len > 0 && edit_tmpdir[--len] == '/') + edit_tmpdir[len] = '\0'; + debug_return_bool(true); +} + static void switch_user(uid_t euid, gid_t egid, int ngroups, GETGROUPS_T *groups) { @@ -80,75 +126,51 @@ switch_user(uid_t euid, gid_t egid, int ngroups, GETGROUPS_T *groups) } /* - * Wrapper to allow users to edit privileged files with their own uid. + * Construct a temporary file name for file and return an + * open file descriptor. The temporary file name is stored + * in tfile which the caller is responsible for freeing. */ -int -sudo_edit(struct command_details *command_details) +static int +sudo_edit_mktemp(const char *ofile, char **tfile) { - struct command_details saved_command_details; - ssize_t nread, nwritten; - const char *tmpdir; - char *cp, *suff, **nargv = NULL, **ap, **files = NULL; - char buf[BUFSIZ]; - int rc, i, j, ac, ofd, tfd, nargc, rval, tmplen; - int editor_argc = 0, nfiles = 0; - struct stat sb; - struct timeval tv, times[2]; - struct tempfile { - char *tfile; - char *ofile; - struct timeval omtim; - off_t osize; - } *tf = NULL; - debug_decl(sudo_edit, SUDO_DEBUG_EDIT) + const char *cp, *suff; + debug_decl(sudo_edit_mktemp, SUDO_DEBUG_EDIT) - /* - * Set real, effective and saved uids to root. - * We will change the euid as needed below. - */ - if (setuid(ROOT_UID) != 0) { - sudo_warn(U_("unable to change uid to root (%u)"), ROOT_UID); - goto cleanup; - } - - /* - * Find our temporary directory, one of /var/tmp, /usr/tmp, or /tmp - */ - if (stat(_PATH_VARTMP, &sb) == 0 && S_ISDIR(sb.st_mode)) - tmpdir = _PATH_VARTMP; -#ifdef _PATH_USRTMP - else if (stat(_PATH_USRTMP, &sb) == 0 && S_ISDIR(sb.st_mode)) - tmpdir = _PATH_USRTMP; -#endif + if ((cp = strrchr(ofile, '/')) != NULL) + cp++; else - tmpdir = _PATH_TMP; - tmplen = strlen(tmpdir); - while (tmplen > 0 && tmpdir[tmplen - 1] == '/') - tmplen--; + cp = ofile; + suff = strrchr(cp, '.'); + if (suff != NULL) { + sudo_easprintf(tfile, "%s/%.*sXXXXXXXX%s", edit_tmpdir, + (int)(size_t)(suff - cp), cp, suff); + } else { + sudo_easprintf(tfile, "%s/%s.XXXXXXXX", edit_tmpdir, cp); + } + debug_return_int(mkstemps(*tfile, suff ? strlen(suff) : 0)); +} - /* - * The user's editor must be separated from the files to be - * edited by a "--" option. - */ - for (ap = command_details->argv; *ap != NULL; ap++) { - if (files) - nfiles++; - else if (strcmp(*ap, "--") == 0) - files = ap + 1; - else - editor_argc++; - } - if (nfiles == 0) { - sudo_warnx(U_("plugin error: missing file list for sudoedit")); - goto cleanup; - } +/* + * Create temporary copies of files[] and store the temporary path name + * along with the original name, size and mtime in tf. + * Returns the number of files copied (which may be less than nfiles) + * or -1 if a fatal error occurred. + */ +static int +sudo_edit_create_tfiles(struct command_details *command_details, + struct tempfile *tf, char * const files[], int nfiles) +{ + int i, j, tfd, ofd, rc; + char buf[BUFSIZ]; + ssize_t nwritten, nread; + struct timeval times[2]; + struct stat sb; + debug_decl(sudo_edit_create_tfiles, SUDO_DEBUG_EDIT) /* * For each file specified by the user, make a temporary version * and copy the contents of the original to it. */ - tf = sudo_emallocarray(nfiles, sizeof(*tf)); - memset(tf, 0, nfiles * sizeof(*tf)); for (i = 0, j = 0; i < nfiles; i++) { rc = -1; switch_user(command_details->euid, command_details->egid, @@ -175,25 +197,14 @@ sudo_edit(struct command_details *command_details) tf[j].ofile = files[i]; tf[j].osize = sb.st_size; mtim_get(&sb, &tf[j].omtim); - if ((cp = strrchr(tf[j].ofile, '/')) != NULL) - cp++; - else - cp = tf[j].ofile; - suff = strrchr(cp, '.'); - if (suff != NULL) { - sudo_easprintf(&tf[j].tfile, "%.*s/%.*sXXXXXXXX%s", tmplen, tmpdir, - (int)(size_t)(suff - cp), cp, suff); - } else { - sudo_easprintf(&tf[j].tfile, "%.*s/%s.XXXXXXXX", tmplen, tmpdir, cp); - } if (seteuid(user_details.uid) != 0) sudo_fatal("seteuid(%d)", (int)user_details.uid); - tfd = mkstemps(tf[j].tfile, suff ? strlen(suff) : 0); + tfd = sudo_edit_mktemp(tf[j].ofile, &tf[j].tfile); if (seteuid(ROOT_UID) != 0) sudo_fatal("seteuid(ROOT_UID)"); if (tfd == -1) { sudo_warn("mkstemps"); - goto cleanup; + debug_return_int(-1); } if (ofd != -1) { while ((nread = read(ofd, buf, sizeof(buf))) != 0) { @@ -202,7 +213,7 @@ sudo_edit(struct command_details *command_details) sudo_warn("%s", tf[j].tfile); else sudo_warnx(U_("%s: short write"), tf[j].tfile); - goto cleanup; + debug_return_int(-1); } } close(ofd); @@ -227,8 +238,143 @@ sudo_edit(struct command_details *command_details) close(tfd); j++; } - if ((nfiles = j) == 0) - goto cleanup; /* no files readable, you lose */ + debug_return_int(j); +} + +/* + * Copy the temporary files specified in tf to the originals. + * Returns the number of copy errors or 0 if completely successful. + */ +static int +sudo_edit_copy_tfiles(struct command_details *command_details, + struct tempfile *tf, int nfiles, struct timeval *times) +{ + int i, tfd, ofd, rc, errors = 0; + char buf[BUFSIZ]; + ssize_t nwritten, nread; + struct timeval tv; + struct stat sb; + debug_decl(sudo_edit_copy_tfiles, SUDO_DEBUG_EDIT) + + /* Copy contents of temp files to real ones. */ + for (i = 0; i < nfiles; i++) { + rc = -1; + if (seteuid(user_details.uid) != 0) + sudo_fatal("seteuid(%d)", (int)user_details.uid); + if ((tfd = open(tf[i].tfile, O_RDONLY, 0644)) != -1) { + rc = fstat(tfd, &sb); + } + if (seteuid(ROOT_UID) != 0) + sudo_fatal("seteuid(ROOT_UID)"); + if (rc || !S_ISREG(sb.st_mode)) { + if (rc) + sudo_warn("%s", tf[i].tfile); + else + sudo_warnx(U_("%s: not a regular file"), tf[i].tfile); + sudo_warnx(U_("%s left unmodified"), tf[i].ofile); + if (tfd != -1) + close(tfd); + errors++; + continue; + } + mtim_get(&sb, &tv); + if (tf[i].osize == sb.st_size && sudo_timevalcmp(&tf[i].omtim, &tv, ==)) { + /* + * If mtime and size match but the user spent no measurable + * time in the editor we can't tell if the file was changed. + */ + if (sudo_timevalcmp(×[0], ×[1], !=)) { + sudo_warnx(U_("%s unchanged"), tf[i].ofile); + unlink(tf[i].tfile); + close(tfd); + errors++; + continue; + } + } + switch_user(command_details->euid, command_details->egid, + command_details->ngroups, command_details->groups); + ofd = open(tf[i].ofile, O_WRONLY|O_TRUNC|O_CREAT, 0644); + switch_user(ROOT_UID, user_details.egid, + user_details.ngroups, user_details.groups); + if (ofd == -1) { + sudo_warn(U_("unable to write to %s"), tf[i].ofile); + sudo_warnx(U_("contents of edit session left in %s"), tf[i].tfile); + close(tfd); + errors++; + continue; + } + while ((nread = read(tfd, buf, sizeof(buf))) > 0) { + if ((nwritten = write(ofd, buf, nread)) != nread) { + if (nwritten == -1) + sudo_warn("%s", tf[i].ofile); + else + sudo_warnx(U_("%s: short write"), tf[i].ofile); + break; + } + } + if (nread == 0) { + /* success, got EOF */ + unlink(tf[i].tfile); + } else if (nread < 0) { + sudo_warn(U_("unable to read temporary file")); + sudo_warnx(U_("contents of edit session left in %s"), tf[i].tfile); + } else { + sudo_warn(U_("unable to write to %s"), tf[i].ofile); + sudo_warnx(U_("contents of edit session left in %s"), tf[i].tfile); + } + close(ofd); + } + debug_return_int(errors); +} + +/* + * Wrapper to allow users to edit privileged files with their own uid. + */ +int +sudo_edit(struct command_details *command_details) +{ + struct command_details saved_command_details; + char **nargv = NULL, **ap, **files = NULL; + int i, ac, nargc, rval; + int editor_argc = 0, nfiles = 0; + struct timeval times[2]; + struct tempfile *tf = NULL; + debug_decl(sudo_edit, SUDO_DEBUG_EDIT) + + if (!set_tmpdir()) + goto cleanup; + + /* + * Set real, effective and saved uids to root. + * We will change the euid as needed below. + */ + if (setuid(ROOT_UID) != 0) { + sudo_warn(U_("unable to change uid to root (%u)"), ROOT_UID); + goto cleanup; + } + + /* + * The user's editor must be separated from the files to be + * edited by a "--" option. + */ + for (ap = command_details->argv; *ap != NULL; ap++) { + if (files) + nfiles++; + else if (strcmp(*ap, "--") == 0) + files = ap + 1; + else + editor_argc++; + } + if (nfiles == 0) { + sudo_warnx(U_("plugin error: missing file list for sudoedit")); + goto cleanup; + } + + /* Copy editor files to temporaries. */ + tf = sudo_ecalloc(nfiles, sizeof(*tf)); + nfiles = sudo_edit_create_tfiles(command_details, tf, files, nfiles); + if (nfiles <= 0) + goto cleanup; /* * Allocate space for the new argument vector and fill it in. @@ -269,70 +415,9 @@ sudo_edit(struct command_details *command_details) command_details->argv = saved_command_details.argv; /* Copy contents of temp files to real ones. */ - for (i = 0; i < nfiles; i++) { - rc = -1; - if (seteuid(user_details.uid) != 0) - sudo_fatal("seteuid(%d)", (int)user_details.uid); - if ((tfd = open(tf[i].tfile, O_RDONLY, 0644)) != -1) { - rc = fstat(tfd, &sb); - } - if (seteuid(ROOT_UID) != 0) - sudo_fatal("seteuid(ROOT_UID)"); - if (rc || !S_ISREG(sb.st_mode)) { - if (rc) - sudo_warn("%s", tf[i].tfile); - else - sudo_warnx(U_("%s: not a regular file"), tf[i].tfile); - sudo_warnx(U_("%s left unmodified"), tf[i].ofile); - if (tfd != -1) - close(tfd); - continue; - } - mtim_get(&sb, &tv); - if (tf[i].osize == sb.st_size && sudo_timevalcmp(&tf[i].omtim, &tv, ==)) { - /* - * If mtime and size match but the user spent no measurable - * time in the editor we can't tell if the file was changed. - */ - if (sudo_timevalcmp(×[0], ×[1], !=)) { - sudo_warnx(U_("%s unchanged"), tf[i].ofile); - unlink(tf[i].tfile); - close(tfd); - continue; - } - } - switch_user(command_details->euid, command_details->egid, - command_details->ngroups, command_details->groups); - ofd = open(tf[i].ofile, O_WRONLY|O_TRUNC|O_CREAT, 0644); - switch_user(ROOT_UID, user_details.egid, - user_details.ngroups, user_details.groups); - if (ofd == -1) { - sudo_warn(U_("unable to write to %s"), tf[i].ofile); - sudo_warnx(U_("contents of edit session left in %s"), tf[i].tfile); - close(tfd); - continue; - } - while ((nread = read(tfd, buf, sizeof(buf))) > 0) { - if ((nwritten = write(ofd, buf, nread)) != nread) { - if (nwritten == -1) - sudo_warn("%s", tf[i].ofile); - else - sudo_warnx(U_("%s: short write"), tf[i].ofile); - break; - } - } - if (nread == 0) { - /* success, got EOF */ - unlink(tf[i].tfile); - } else if (nread < 0) { - sudo_warn(U_("unable to read temporary file")); - sudo_warnx(U_("contents of edit session left in %s"), tf[i].tfile); - } else { - sudo_warn(U_("unable to write to %s"), tf[i].ofile); - sudo_warnx(U_("contents of edit session left in %s"), tf[i].tfile); - } - close(ofd); - } + if (sudo_edit_copy_tfiles(command_details, tf, nfiles, times) != 0) + rval = 1; + sudo_efree(tf); sudo_efree(nargv); debug_return_int(rval);