Use sudo_secure_open_file() instead of sudo_secure_file() where possible.

Both sudo_secure_open_file() and sudo_secure_open_dir() are now passed
a struct stat pointer like sudo_secure_file() and sudo_secure_dir().

9 files changed, 145 insertions, 128 deletions

diff --git a/docs/sudoers.man.in b/docs/sudoers.man.in
index 4702f2cd4..abe22b5af 100644
--- a/docs/sudoers.man.in
+++ b/docs/sudoers.man.in
@@ -5950,7 +5950,7 @@ line in the
 sudo.conf(@mansectform@)
 file.
 .TP 3n
-unable to stat @sysconfdir@/sudoers
+unable to open @sysconfdir@/sudoers
 The
 \fI@sysconfdir@/sudoers\fR
 file is missing.
diff --git a/docs/sudoers.mdoc.in b/docs/sudoers.mdoc.in
index ddf08db3f..4aeb92004 100644
--- a/docs/sudoers.mdoc.in
+++ b/docs/sudoers.mdoc.in
@@ -5557,7 +5557,7 @@ file) to the end of the
 line in the
 .Xr sudo.conf @mansectform@
 file.
-.It unable to stat @sysconfdir@/sudoers
+.It unable to open @sysconfdir@/sudoers
 The
 .Pa @sysconfdir@/sudoers
 file is missing.
diff --git a/include/sudo_util.h b/include/sudo_util.h
index 1ccb52d5e..911b00b8c 100644
--- a/include/sudo_util.h
+++ b/include/sudo_util.h
@@ -293,14 +293,14 @@ sudo_dso_public unsigned int sudo_pow2_roundup_v1(unsigned int len);
 #define SUDO_PATH_WORLD_WRITABLE	-4
 #define SUDO_PATH_GROUP_WRITABLE	-5
 struct stat;
-sudo_dso_public int sudo_secure_dir_v1(const char *path, uid_t uid, gid_t gid, struct stat *sbp);
+sudo_dso_public int sudo_secure_dir_v1(const char *path, uid_t uid, gid_t gid, struct stat *sb);
 #define sudo_secure_dir(_a, _b, _c, _d) sudo_secure_dir_v1((_a), (_b), (_c), (_d))
-sudo_dso_public int sudo_secure_file_v1(const char *path, uid_t uid, gid_t gid, struct stat *sbp);
+sudo_dso_public int sudo_secure_file_v1(const char *path, uid_t uid, gid_t gid, struct stat *sb);
 #define sudo_secure_file(_a, _b, _c, _d) sudo_secure_file_v1((_a), (_b), (_c), (_d))
-sudo_dso_public int sudo_secure_open_file_v1(const char *path, uid_t uid, gid_t gid, int *error);
-#define sudo_secure_open_file(_a, _b, _c, _d) sudo_secure_open_file_v1((_a), (_b), (_c), (_d))
-sudo_dso_public int sudo_secure_open_dir_v1(const char *path, uid_t uid, gid_t gid, int *error);
-#define sudo_secure_open_dir(_a, _b, _c, _d) sudo_secure_open_dir_v1((_a), (_b), (_c), (_d))
+sudo_dso_public int sudo_secure_open_file_v1(const char *path, uid_t uid, gid_t gid, struct stat *sb, int *error);
+#define sudo_secure_open_file(_a, _b, _c, _d, _e) sudo_secure_open_file_v1((_a), (_b), (_c), (_d), (_e))
+sudo_dso_public int sudo_secure_open_dir_v1(const char *path, uid_t uid, gid_t gid, struct stat *sb, int *error);
+#define sudo_secure_open_dir(_a, _b, _c, _d, _e) sudo_secure_open_dir_v1((_a), (_b), (_c), (_d), (_e))
 
 /* setgroups.c */
 sudo_dso_public int sudo_setgroups_v1(int ngids, const GETGROUPS_T *gids);
diff --git a/lib/util/secure_path.c b/lib/util/secure_path.c
index d2438f78d..89d21deee 100644
--- a/lib/util/secure_path.c
+++ b/lib/util/secure_path.c
@@ -77,6 +77,7 @@ sudo_secure_path(const char *path, unsigned int type, uid_t uid, gid_t gid,
 
 /*
  * Verify that path is a regular file and not writable by other users.
+ * Not currently used.
  */
 int
 sudo_secure_file_v1(const char *path, uid_t uid, gid_t gid, struct stat *sb)
@@ -99,21 +100,25 @@ sudo_secure_dir_v1(const char *path, uid_t uid, gid_t gid, struct stat *sb)
  * Sets error to SUDO_PATH_SECURE on success, and a value < 0 on failure.
  */
 static int
-sudo_secure_open(const char *path, int type, uid_t uid, gid_t gid, int *error)
+sudo_secure_open(const char *path, int type, uid_t uid, gid_t gid,
+    struct stat *sb, int *error)
 {
-    struct stat sb;
+    struct stat stat_buf;
     int fd;
     debug_decl(sudo_secure_open, SUDO_DEBUG_UTIL);
 
+    if (sb == NULL)
+	sb = &stat_buf;
+
     fd = open(path, O_RDONLY|O_NONBLOCK);
-    if (fd == -1 || fstat(fd, &sb) != 0) {
+    if (fd == -1 || fstat(fd, sb) != 0) {
 	if (fd != -1)
 	    close(fd);
 	*error = SUDO_PATH_MISSING;
 	debug_return_int(-1);
     }
 
-    *error = sudo_check_secure(&sb, type, uid, gid);
+    *error = sudo_check_secure(sb, type, uid, gid);
     if (*error == SUDO_PATH_SECURE) {
 	(void)fcntl(fd, F_SETFL, fcntl(fd, F_GETFL, 0) & ~O_NONBLOCK);
     } else {
@@ -126,13 +131,15 @@ sudo_secure_open(const char *path, int type, uid_t uid, gid_t gid, int *error)
 }
 
 int
-sudo_secure_open_file_v1(const char *path, uid_t uid, gid_t gid, int *error)
+sudo_secure_open_file_v1(const char *path, uid_t uid, gid_t gid,
+    struct stat *sb, int *error)
 {
-    return sudo_secure_open(path, S_IFREG, uid, gid, error);
+    return sudo_secure_open(path, S_IFREG, uid, gid, sb, error);
 }
 
 int
-sudo_secure_open_dir_v1(const char *path, uid_t uid, gid_t gid, int *error)
+sudo_secure_open_dir_v1(const char *path, uid_t uid, gid_t gid,
+    struct stat *sb, int *error)
 {
-    return sudo_secure_open(path, S_IFDIR, uid, gid, error);
+    return sudo_secure_open(path, S_IFDIR, uid, gid, sb, error);
 }
diff --git a/lib/util/sudo_conf.c b/lib/util/sudo_conf.c
index 5357df1d3..454d071d2 100644
--- a/lib/util/sudo_conf.c
+++ b/lib/util/sudo_conf.c
@@ -38,6 +38,7 @@
 #include <unistd.h>
 #include <ctype.h>
 #include <errno.h>
+#include <fcntl.h>
 #include <limits.h>
 
 #define SUDO_ERROR_WRAP	0
@@ -651,7 +652,7 @@ sudo_conf_read_v1(const char *conf_file, int conf_types)
 {
     struct stat sb;
     FILE *fp = NULL;
-    int ret = false;
+    int fd, ret = false;
     char *prev_locale, *line = NULL;
     unsigned int conf_lineno = 0;
     size_t linesize = 0;
@@ -670,38 +671,48 @@ sudo_conf_read_v1(const char *conf_file, int conf_types)
     if (prev_locale[0] != 'C' || prev_locale[1] != '\0')
         setlocale(LC_ALL, "C");
 
-    if (conf_file == NULL) {
+    if (conf_file != NULL) {
+	fd = open(conf_file, O_RDONLY);
+	if (fd == -1) {
+	    sudo_warn(U_("unable to open %s"), conf_file);
+	    goto done;
+	}
+    } else {
+	int error;
 	conf_file = _PATH_SUDO_CONF;
-	switch (sudo_secure_file(conf_file, ROOT_UID, -1, &sb)) {
-	    case SUDO_PATH_SECURE:
-		break;
-	    case SUDO_PATH_MISSING:
-		/* Root should always be able to read sudo.conf. */
-		if (errno != ENOENT && geteuid() == ROOT_UID)
-		    sudo_warn(U_("unable to stat %s"), conf_file);
-		goto done;
-	    case SUDO_PATH_BAD_TYPE:
-		sudo_warnx(U_("%s is not a regular file"), conf_file);
-		goto done;
-	    case SUDO_PATH_WRONG_OWNER:
-		sudo_warnx(U_("%s is owned by uid %u, should be %u"),
-		    conf_file, (unsigned int) sb.st_uid, ROOT_UID);
-		goto done;
-	    case SUDO_PATH_WORLD_WRITABLE:
-		sudo_warnx(U_("%s is world writable"), conf_file);
-		goto done;
-	    case SUDO_PATH_GROUP_WRITABLE:
-		sudo_warnx(U_("%s is group writable"), conf_file);
-		goto done;
-	    default:
-		/* NOTREACHED */
-		goto done;
+	fd = sudo_secure_open_file(conf_file, ROOT_UID, -1, &sb, &error);
+	switch (error) {
+	case SUDO_PATH_SECURE:
+	    break;
+	case SUDO_PATH_MISSING:
+	    /* Root should always be able to read sudo.conf. */
+	    if (errno != ENOENT && geteuid() == ROOT_UID)
+		sudo_warn(U_("unable to open %s"), conf_file);
+	    goto done;
+	case SUDO_PATH_BAD_TYPE:
+	    sudo_warnx(U_("%s is not a regular file"), conf_file);
+	    goto done;
+	case SUDO_PATH_WRONG_OWNER:
+	    sudo_warnx(U_("%s is owned by uid %u, should be %u"),
+		conf_file, (unsigned int) sb.st_uid, ROOT_UID);
+	    goto done;
+	case SUDO_PATH_WORLD_WRITABLE:
+	    sudo_warnx(U_("%s is world writable"), conf_file);
+	    goto done;
+	case SUDO_PATH_GROUP_WRITABLE:
+	    sudo_warnx(U_("%s is group writable"), conf_file);
+	    goto done;
+	default:
+	    sudo_warnx("%s: internal error, unexpected error %d",
+		__func__, error);
+	    goto done;
 	}
     }
 
-    if ((fp = fopen(conf_file, "r")) == NULL) {
+    if ((fp = fdopen(fd, "r")) == NULL) {
 	if (errno != ENOENT && geteuid() == ROOT_UID)
 	    sudo_warn(U_("unable to open %s"), conf_file);
+	close(fd);
 	goto done;
     }
 
diff --git a/plugins/sudoers/regress/testsudoers/test11.out.ok b/plugins/sudoers/regress/testsudoers/test11.out.ok
index 987ab6559..f030dc84d 100644
--- a/plugins/sudoers/regress/testsudoers/test11.out.ok
+++ b/plugins/sudoers/regress/testsudoers/test11.out.ok
@@ -3,7 +3,7 @@ Testing @include with garbage after the path name
 sudoers:1:24: syntax error
 @include sudoers.local womp womp
                        ^~~~
-testsudoers: unable to stat sudoers.local: No such file or directory
+testsudoers: unable to open sudoers.local: No such file or directory
 
 Entries for user root:
 
@@ -14,7 +14,7 @@ Testing #include with garbage after the path name
 sudoers:1:24: syntax error
 #include sudoers.local womp womp
                        ^~~~
-testsudoers: unable to stat sudoers.local: No such file or directory
+testsudoers: unable to open sudoers.local: No such file or directory
 
 Entries for user root:
 
diff --git a/plugins/sudoers/sudoers.c b/plugins/sudoers/sudoers.c
index 33d3398c3..abe63147d 100644
--- a/plugins/sudoers/sudoers.c
+++ b/plugins/sudoers/sudoers.c
@@ -1093,36 +1093,25 @@ set_cmnd(void)
 FILE *
 open_sudoers(const char *file, bool doedit, bool *keepopen)
 {
-    struct stat sb;
     FILE *fp = NULL;
-    bool perm_root = false;
+    struct stat sb;
+    int error, fd;
     debug_decl(open_sudoers, SUDOERS_DEBUG_PLUGIN);
 
     if (!set_perms(PERM_SUDOERS))
 	debug_return_ptr(NULL);
 
 again:
-    switch (sudo_secure_file(file, sudoers_uid, sudoers_gid, &sb)) {
+    fd = sudo_secure_open_file(file, sudoers_uid, sudoers_gid, &sb, &error);
+    switch (error) {
 	case SUDO_PATH_SECURE:
 	    /*
-	     * If we are expecting sudoers to be group readable by
-	     * SUDOERS_GID but it is not, we must open the file as root,
-	     * not uid 1.
-	     */
-	    if (sudoers_uid == ROOT_UID && ISSET(sudoers_mode, S_IRGRP)) {
-		if (!ISSET(sb.st_mode, S_IRGRP) || sb.st_gid != SUDOERS_GID) {
-		    if (!perm_root) {
-			if (!restore_perms() || !set_perms(PERM_ROOT))
-			    debug_return_ptr(NULL);
-		    }
-		}
-	    }
-	    /*
-	     * Open file and make sure we can read it so we can present
-	     * the user with a reasonable error message (unlike the lexer).
+	     * Make sure we can read the file so we can present the
+	     * user with a reasonable error message (unlike the lexer).
 	     */
-	    if ((fp = fopen(file, "r")) == NULL) {
+	    if ((fp = fdopen(fd, "r")) == NULL) {
 		log_warning(SLOG_SEND_MAIL, N_("unable to open %s"), file);
+		close(fd);
 	    } else {
 		if (sb.st_size != 0 && fgetc(fp) == EOF) {
 		    log_warning(SLOG_SEND_MAIL,
@@ -1138,7 +1127,7 @@ again:
 	    break;
 	case SUDO_PATH_MISSING:
 	    /*
-	     * If we tried to stat() sudoers as non-root but got EACCES,
+	     * If we tried to open sudoers as non-root but got EACCES,
 	     * try again as root.
 	     */
 	    if (errno == EACCES && geteuid() != ROOT_UID) {
@@ -1146,12 +1135,11 @@ again:
 		if (restore_perms()) {
 		    if (!set_perms(PERM_ROOT))
 			debug_return_ptr(NULL);
-		    perm_root = true;
 		    goto again;
 		}
 		errno = serrno;
 	    }
-	    log_warning(SLOG_SEND_MAIL, N_("unable to stat %s"), file);
+	    log_warning(SLOG_SEND_MAIL, N_("unable to open %s"), file);
 	    break;
 	case SUDO_PATH_BAD_TYPE:
 	    log_warningx(SLOG_SEND_MAIL,
diff --git a/plugins/sudoers/testsudoers.c b/plugins/sudoers/testsudoers.c
index 8fa7d9d8d..415c723dc 100644
--- a/plugins/sudoers/testsudoers.c
+++ b/plugins/sudoers/testsudoers.c
@@ -442,34 +442,37 @@ open_sudoers(const char *file, bool doedit, bool *keepopen)
     struct stat sb;
     FILE *fp = NULL;
     const char *base;
+    int error, fd;
     debug_decl(open_sudoers, SUDOERS_DEBUG_UTIL);
 
     /* Report errors using the basename for consistent test output. */
     base = sudo_basename(file);
-    switch (sudo_secure_file(file, sudoers_uid, sudoers_gid, &sb)) {
-	case SUDO_PATH_SECURE:
-	    fp = fopen(file, "r");
-	    break;
-	case SUDO_PATH_MISSING:
-	    sudo_warn("unable to stat %s", base);
-	    break;
-	case SUDO_PATH_BAD_TYPE:
-	    sudo_warnx("%s is not a regular file", base);
-	    break;
-	case SUDO_PATH_WRONG_OWNER:
-	    sudo_warnx("%s should be owned by uid %u",
-		base, (unsigned int) sudoers_uid);
-	    break;
-	case SUDO_PATH_WORLD_WRITABLE:
-	    sudo_warnx("%s is world writable", base);
-	    break;
-	case SUDO_PATH_GROUP_WRITABLE:
-	    sudo_warnx("%s should be owned by gid %u",
-		base, (unsigned int) sudoers_gid);
-	    break;
-	default:
-	    /* NOTREACHED */
-	    break;
+    fd = sudo_secure_open_file(file, sudoers_uid, sudoers_gid, &sb, &error);
+    switch (error) {
+    case SUDO_PATH_SECURE:
+	if ((fp = fdopen(fd, "r")) == NULL)
+	    close(fd);
+	break;
+    case SUDO_PATH_MISSING:
+	sudo_warn("unable to open %s", base);
+	break;
+    case SUDO_PATH_BAD_TYPE:
+	sudo_warnx("%s is not a regular file", base);
+	break;
+    case SUDO_PATH_WRONG_OWNER:
+	sudo_warnx("%s should be owned by uid %u",
+	    base, (unsigned int) sudoers_uid);
+	break;
+    case SUDO_PATH_WORLD_WRITABLE:
+	sudo_warnx("%s is world writable", base);
+	break;
+    case SUDO_PATH_GROUP_WRITABLE:
+	sudo_warnx("%s should be owned by gid %u",
+	    base, (unsigned int) sudoers_gid);
+	break;
+    default:
+	/* NOTREACHED */
+	break;
     }
 
     debug_return_ptr(fp);
diff --git a/plugins/sudoers/timestamp.c b/plugins/sudoers/timestamp.c
index 155cc0fec..146fd3b33 100644
--- a/plugins/sudoers/timestamp.c
+++ b/plugins/sudoers/timestamp.c
@@ -236,47 +236,55 @@ static int
 ts_secure_opendir(const char *path, bool make_it, bool quiet)
 {
     int error, fd = -1;
+    struct stat sb;
     debug_decl(ts_secure_opendir, SUDOERS_DEBUG_AUTH);
 
     sudo_debug_printf(SUDO_DEBUG_INFO|SUDO_DEBUG_LINENO, "checking %s", path);
-    fd = sudo_secure_open_dir(path, timestamp_uid, timestamp_gid, &error);
-    if (fd == -1) {
-	switch (error) {
-	case SUDO_PATH_MISSING:
-	    if (make_it) {
-		fd = ts_mkdirs(path, timestamp_uid, timestamp_gid, S_IRWXU,
-		    S_IRWXU|S_IXGRP|S_IXOTH, quiet);
-		if (fd != -1)
-		    break;
-	    }
-	    if (!quiet)
-		sudo_warn("%s", path);
-	    break;
-	case SUDO_PATH_BAD_TYPE:
-	    errno = ENOTDIR;
-	    if (!quiet)
-		sudo_warn("%s", path);
-	    break;
-	case SUDO_PATH_WRONG_OWNER:
-	    if (!quiet) {
-		sudo_warnx(U_("%s: wrong owner, should be uid %u\n"),
-		    path, (unsigned int)timestamp_uid);
-	    }
-	    errno = EACCES;
-	    break;
-	case SUDO_PATH_GROUP_WRITABLE:
-	    if (!quiet)
-		sudo_warnx(U_("%s is group writable"), path);
-	    errno = EACCES;
-	    break;
-	default:
-	    if (!quiet) {
-		sudo_warnx("%s: internal error, unexpected error %d",
-		    __func__, error);
-		errno = EINVAL;
-	    }
-	    break;
+    fd = sudo_secure_open_dir(path, timestamp_uid, timestamp_gid, &sb, &error);
+    switch (error) {
+    case SUDO_PATH_SECURE:
+	break;
+    case SUDO_PATH_MISSING:
+	if (make_it) {
+	    fd = ts_mkdirs(path, timestamp_uid, timestamp_gid, S_IRWXU,
+		S_IRWXU|S_IXGRP|S_IXOTH, quiet);
+	    if (fd != -1)
+		break;
+	}
+	if (!quiet)
+	    sudo_warn("%s", path);
+	break;
+    case SUDO_PATH_BAD_TYPE:
+	errno = ENOTDIR;
+	if (!quiet)
+	    sudo_warn("%s", path);
+	break;
+    case SUDO_PATH_WRONG_OWNER:
+	if (!quiet) {
+	    sudo_warnx(U_("%s is owned by uid %u, should be %u"),
+		path, (unsigned int)sb.st_uid, (unsigned int)timestamp_uid);
+	}
+	errno = EACCES;
+	break;
+    case SUDO_PATH_WORLD_WRITABLE:
+	if (!quiet)
+	    sudo_warnx(U_("%s is world writable"), path);
+	errno = EACCES;
+	break;
+    case SUDO_PATH_GROUP_WRITABLE:
+	if (!quiet) {
+	    sudo_warnx(U_("%s is owned by gid %u, should be %u"),
+		path, (unsigned int)sb.st_gid, (unsigned int)timestamp_gid);
+	}
+	errno = EACCES;
+	break;
+    default:
+	if (!quiet) {
+	    sudo_warnx("%s: internal error, unexpected error %d",
+		__func__, error);
+	    errno = EINVAL;
 	}
+	break;
     }
 
     debug_return_int(fd);