From: Todd C. Miller <Todd.Miller@courtesan.com>
Date: Tue, 21 Mar 2017 22:21:17 +0000 (-0600)
Subject: Set umask temporarily when creating files instead of changing the
X-Git-Tag: SUDO_1_8_20^2~65
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=7668b4b42bd04cd99178c1c80260860910946ac0;p=sudo

Set umask temporarily when creating files instead of changing the
mode after the fact.  This is slightly less error prone.
---

diff --git a/plugins/sudoers/iolog.c b/plugins/sudoers/iolog.c
index 9687b6d24..924fa88a9 100644
--- a/plugins/sudoers/iolog.c
+++ b/plugins/sudoers/iolog.c
@@ -130,7 +130,6 @@ io_mkdirs(char *path)
 	    ok = false;
 	} else {
 	    ignore_result(chown(path, iolog_uid, iolog_gid));
-	    ignore_result(chmod(path, iolog_dirmode));
 	}
     }
     if (uid_changed) {
@@ -340,12 +339,16 @@ io_nextid(char *iolog_dir, char *iolog_dir_fallback, char sessid[7])
     char buf[32], *ep;
     int i, len, fd = -1;
     unsigned long id = 0;
+    mode_t omask;
     ssize_t nread;
     bool ret = false;
     char pathbuf[PATH_MAX];
     static const char b36char[] = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";
     debug_decl(io_nextid, SUDOERS_DEBUG_UTIL)
 
+    /* umask must not be more restrictive than the file modes. */
+    omask = umask(ACCESSPERMS & ~(iolog_filemode|iolog_dirmode));
+
     /*
      * Create I/O log directory if it doesn't already exist.
      */
@@ -374,7 +377,6 @@ io_nextid(char *iolog_dir, char *iolog_dir_fallback, char sessid[7])
     }
     sudo_lock_file(fd, SUDO_LOCK);
     ignore_result(fchown(fd, iolog_uid, iolog_gid));
-    ignore_result(fchmod(fd, iolog_filemode));
 
     /*
      * If there is no seq file in iolog_dir and a fallback dir was
@@ -398,7 +400,6 @@ io_nextid(char *iolog_dir, char *iolog_dir_fallback, char sessid[7])
 	    }
 	    if (fd2 != -1) {
 		ignore_result(fchown(fd2, iolog_uid, iolog_gid));
-		ignore_result(fchmod(fd2, iolog_filemode));
 		nread = read(fd2, buf, sizeof(buf) - 1);
 		if (nread > 0) {
 		    if (buf[nread - 1] == '\n')
@@ -464,6 +465,7 @@ io_nextid(char *iolog_dir, char *iolog_dir_fallback, char sessid[7])
     ret = true;
 
 done:
+    umask(omask);
     if (fd != -1)
 	close(fd);
     debug_return_bool(ret);
@@ -524,7 +526,6 @@ open_io_fd(char *pathbuf, size_t len, struct io_log_file *iol, bool docompress)
 	}
 	if (fd != -1) {
 	    ignore_result(fchown(fd, iolog_uid, iolog_gid));
-	    ignore_result(fchmod(fd, iolog_filemode));
 	    (void)fcntl(fd, F_SETFD, FD_CLOEXEC);
 #ifdef HAVE_ZLIB_H
 	    if (docompress)
@@ -767,7 +768,6 @@ write_info_log(char *pathbuf, size_t len, struct iolog_details *details,
 	debug_return_bool(false);
     }
     ignore_result(fchown(fd, iolog_uid, iolog_gid));
-    ignore_result(fchmod(fd, iolog_filemode));
 
     fprintf(fp, "%lld:%s:%s:%s:%s:%d:%d\n%s\n%s", (long long)now->tv_sec,
 	details->user ? details->user : "unknown", details->runas_pw->pw_name,
@@ -850,6 +850,7 @@ sudoers_io_open(unsigned int version, sudo_conv_t conversation,
     char * const *cur;
     const char *cp, *plugin_path = NULL;
     size_t len;
+    mode_t omask;
     int i, ret = -1;
     debug_decl(sudoers_io_open, SUDOERS_DEBUG_PLUGIN)
 
@@ -875,6 +876,10 @@ sudoers_io_open(unsigned int version, sudo_conv_t conversation,
 	    continue;
 	}
     }
+
+    /* umask must not be more restrictive than the file modes. */
+    omask = umask(ACCESSPERMS & ~(iolog_filemode|iolog_dirmode));
+
     if (!sudoers_debug_register(plugin_path, &debug_files)) {
 	ret = -1;
 	goto done;
@@ -943,6 +948,7 @@ sudoers_io_open(unsigned int version, sudo_conv_t conversation,
     ret = true;
 
 done:
+    umask(omask);
     free(tofree);
     if (iolog_details.runas_pw)
 	sudo_pw_delref(iolog_details.runas_pw);
diff --git a/plugins/sudoers/mkdir_parents.c b/plugins/sudoers/mkdir_parents.c
index 87db53f15..335a35704 100644
--- a/plugins/sudoers/mkdir_parents.c
+++ b/plugins/sudoers/mkdir_parents.c
@@ -53,7 +53,6 @@ sudo_mkdir_parents(char *path, uid_t uid, gid_t gid, mode_t mode, bool quiet)
 	if (mkdir(path, mode) == 0) {
 	    if (uid != (uid_t)-1 && gid != (gid_t)-1)
 		ignore_result(chown(path, uid, gid));
-	    ignore_result(chmod(path, mode));
 	} else {
 	    if (errno != EEXIST) {
 		if (!quiet)
diff --git a/plugins/sudoers/timestamp.c b/plugins/sudoers/timestamp.c
index f3c7bf58e..f0a21a5a3 100644
--- a/plugins/sudoers/timestamp.c
+++ b/plugins/sudoers/timestamp.c
@@ -154,8 +154,11 @@ ts_mkdirs(char *path, uid_t owner, gid_t group, mode_t mode,
     mode_t parent_mode, bool quiet)
 {
     bool ret;
+    mode_t omask;
     debug_decl(ts_mkdirs, SUDOERS_DEBUG_AUTH)
 
+    /* umask must not be more restrictive than the file modes. */
+    omask = umask(ACCESSPERMS & ~(mode|parent_mode));
     ret = sudo_mkdir_parents(path, owner, group, parent_mode, quiet); 
     if (ret) {
 	/* Create final path component. */
@@ -170,6 +173,7 @@ ts_mkdirs(char *path, uid_t owner, gid_t group, mode_t mode,
 	    ignore_result(chown(path, owner, group));
 	}
     }
+    umask(omask);
     debug_return_bool(ret);
 }