Add directory writability checks for sudoedit.

NEWS

@@ -74,6 +74,9 @@ What's new in Sudo 1.8.15
 
  * Fixed challenge/response style BSD authentication.
 
+ * Added a sudoers option to prevent sudoedit from editing files
+   located in a directory that is writable by the invoking user.
+
 What's new in Sudo 1.8.14p3
 
  * Fixed a bug introduced in sudo 1.8.14p2 that prevented sudo

config.h.in

@@ -481,6 +481,9 @@
 /* Define to 1 if you have the `nss_search' function. */
 #undef HAVE_NSS_SEARCH
 
+/* Define to 1 if you have the `openat' function. */
+#undef HAVE_OPENAT
+
 /* Define to 1 if you have the `openpty' function. */
 #undef HAVE_OPENPTY
 

configure

@@ -2655,6 +2655,7 @@ as_fn_append ac_func_list " nl_langinfo"
 as_fn_append ac_func_list " strftime"
 as_fn_append ac_func_list " pread"
 as_fn_append ac_func_list " pwrite"
+as_fn_append ac_func_list " openat"
 as_fn_append ac_func_list " seteuid"
 # Check that the precious variables saved in the cache have kept the same
 # value.
@@ -18075,6 +18076,8 @@ done
 
 
 
+
+
 for ac_func in getgrouplist
 do :
   ac_fn_c_check_func "$LINENO" "getgrouplist" "ac_cv_func_getgrouplist"

configure.ac

@@ -2384,7 +2384,7 @@ dnl
 dnl Function checks
 dnl
 AC_FUNC_GETGROUPS
-AC_CHECK_FUNCS_ONCE([killpg nl_langinfo strftime pread pwrite])
+AC_CHECK_FUNCS_ONCE([killpg nl_langinfo strftime pread pwrite openat])
 AC_CHECK_FUNCS([getgrouplist], [], [
     case "$host_os" in
     aix*)

doc/sudoers.cat

@@ -1267,6 +1267,14 @@ SSUUDDOOEERRSS OOPPTTIIOONNSS
                        that support either the setreuid(2) or setresuid(2)
                        system call.  This flag is _o_f_f by default.
 
+     sudoedit_checkdir
+                       If set, ssuuddooeeddiitt will refuse to edit files located in a
+                       directory that is writable by the invoking user unless
+                       it is run by root.  On many systems, this option
+                       requires that the parent directory of the file to be
+                       edited be readable by the target user.  This flag is
+                       _o_f_f by default.
+
      sudoedit_follow   By default, ssuuddooeeddiitt will not follow symbolic links
                        when opening files.  The _s_u_d_o_e_d_i_t___f_o_l_l_o_w option can be
                        enabled to allow ssuuddooeeddiitt to open symbolic links.  It
@@ -2464,4 +2472,4 @@ DDIISSCCLLAAIIMMEERR
      file distributed with ssuuddoo or http://www.sudo.ws/license.html for
      complete details.
 
-Sudo 1.8.15                   September 25, 2015                   Sudo 1.8.15
+Sudo 1.8.15                    October 23, 2015                    Sudo 1.8.15

doc/sudoers.man.in

@@ -21,7 +21,7 @@
 .\" Agency (DARPA) and Air Force Research Laboratory, Air Force
 .\" Materiel Command, USAF, under agreement number F39502-99-1-0512.
 .\"
-.TH "SUDOERS" "5" "September 25, 2015" "Sudo @PACKAGE_VERSION@" "File Formats Manual"
+.TH "SUDOERS" "5" "October 23, 2015" "Sudo @PACKAGE_VERSION@" "File Formats Manual"
 .nh
 .if n .ad l
 .SH "NAME"
@@ -2700,6 +2700,18 @@ This flag is
 \fIoff\fR
 by default.
 .TP 18n
+sudoedit_checkdir
+.br
+If set,
+\fBsudoedit\fR
+will refuse to edit files located in a directory that is writable
+by the invoking user unless it is run by root.
+On many systems, this option requires that the parent directory
+of the file to be edited be readable by the target user.
+This flag is
+\fIoff\fR
+by default.
+.TP 18n
 sudoedit_follow
 By default,
 \fBsudoedit\fR

doc/sudoers.mdoc.in

@@ -19,7 +19,7 @@
 .\" Agency (DARPA) and Air Force Research Laboratory, Air Force
 .\" Materiel Command, USAF, under agreement number F39502-99-1-0512.
 .\"
-.Dd September 25, 2015
+.Dd October 23, 2015
 .Dt SUDOERS @mansectform@
 .Os Sudo @PACKAGE_VERSION@
 .Sh NAME
@@ -2536,6 +2536,16 @@ system call.
 This flag is
 .Em off
 by default.
+.It sudoedit_checkdir
+If set,
+.Nm sudoedit
+will refuse to edit files located in a directory that is writable
+by the invoking user unless it is run by root.
+On many systems, this option requires that the parent directory
+of the file to be edited be readable by the target user.
+This flag is
+.Em off
+by default.
 .It sudoedit_follow
 By default,
 .Nm sudoedit

plugins/sudoers/def_data.c

@@ -386,6 +386,10 @@ struct sudo_defs_types sudo_defs_table[] = {
 	"use_netgroups", T_FLAG,
 	N_("Enable sudoers netgroup support"),
 	NULL,
+    }, {
+	"sudoedit_checkdir", T_FLAG,
+	N_("Check the parent directory for writability when editing files with sudoedit"),
+	NULL,
     }, {
 	"sudoedit_follow", T_FLAG,
 	N_("Follow symbolic links when editing files with sudoedit"),

plugins/sudoers/def_data.h

@@ -180,8 +180,10 @@
 #define I_MAXSEQ                89
 #define def_use_netgroups       (sudo_defs_table[90].sd_un.flag)
 #define I_USE_NETGROUPS         90
-#define def_sudoedit_follow     (sudo_defs_table[91].sd_un.flag)
+#define def_sudoedit_checkdir   (sudo_defs_table[91].sd_un.flag)
-#define I_SUDOEDIT_FOLLOW       91
+#define I_SUDOEDIT_CHECKDIR     91
+#define def_sudoedit_follow     (sudo_defs_table[92].sd_un.flag)
+#define I_SUDOEDIT_FOLLOW       92
 
 enum def_tuple {
 	never,

plugins/sudoers/def_data.in

@@ -286,6 +286,9 @@ maxseq
 use_netgroups
 	T_FLAG
 	"Enable sudoers netgroup support"
+sudoedit_checkdir
+	T_FLAG
+	"Check the parent directory for writability when editing files with sudoedit"
 sudoedit_follow
 	T_FLAG
 	"Follow symbolic links when editing files with sudoedit"

plugins/sudoers/policy.c

@@ -438,6 +438,10 @@ sudoers_policy_exec_setup(char *argv[], char *envp[], mode_t cmnd_umask,
     if (ISSET(sudo_mode, MODE_EDIT)) {
 	if ((command_info[info_len++] = strdup("sudoedit=true")) == NULL)
 	    goto oom;
+	if (def_sudoedit_checkdir) {
+	    if ((command_info[info_len++] = strdup("sudoedit_checkdir=true")) == NULL)
+		goto oom;
+	}
 	if (def_sudoedit_follow) {
 	    if ((command_info[info_len++] = strdup("sudoedit_follow=true")) == NULL)
 		goto oom;

plugins/sudoers/sudoers_version.h

@@ -63,7 +63,7 @@
  * 42	sudo 1.8.6, Support for empty Runas_List (with or without a colon) to mean the invoking user.  Support for Solaris Privilege Sets (PRIVS= and LIMITPRIVS=).
  * 43	sudo 1.8.7, Support for specifying a digest along with the command.
  * 44	sudo 1.8.13, added MAIL/NOMAIL tags.
- * 45	sudo 1.8.15, added FOLLOW/NOFOLLOW tags and sudoedit_follow Default.
+ * 45	sudo 1.8.15, added FOLLOW/NOFOLLOW tags as well as sudoedit_follow and sudoedit_checkdir Defaults.
 */
 
 #ifndef SUDOERS_VERSION_H

src/sudo.c

@@ -727,6 +727,11 @@ command_info_to_details(char * const info[], struct command_details *details)
 			SET(details->flags, CD_SUDOEDIT);
 		    break;
 		}
+		if (strncmp("sudoedit_checkdir=", info[i], sizeof("sudoedit_checkdir=") - 1) == 0) {
+		    if (sudo_strtobool(info[i] + sizeof("sudoedit_checkdir=") - 1) == true)
+			SET(details->flags, CD_SUDOEDIT_CHECKDIR);
+		    break;
+		}
 		if (strncmp("sudoedit_follow=", info[i], sizeof("sudoedit_follow=") - 1) == 0) {
 		    if (sudo_strtobool(info[i] + sizeof("sudoedit_follow=") - 1) == true)
 			SET(details->flags, CD_SUDOEDIT_FOLLOW);

src/sudo.h

@@ -128,6 +128,7 @@ struct user_details {
 #define CD_EXEC_BG		0x04000
 #define CD_SUDOEDIT_COPY	0x08000
 #define CD_SUDOEDIT_FOLLOW	0x10000
+#define CD_SUDOEDIT_CHECKDIR	0x20000
 
 struct preserved_fd {
     TAILQ_ENTRY(preserved_fd) entries;

src/sudo_edit.c

@@ -31,6 +31,7 @@
 #endif /* HAVE_STRINGS_H */
 #include <unistd.h>
 #include <ctype.h>
+#include <errno.h>
 #include <grp.h>
 #include <pwd.h>
 #include <signal.h>
@@ -154,28 +155,180 @@ sudo_edit_mktemp(const char *ofile, char **tfile)
     debug_return_int(tfd);
 }
 
+static bool
+group_matches(gid_t target, gid_t gid, int ngroups, GETGROUPS_T *groups)
+{
+    int i;
+    debug_decl(group_matches, SUDO_DEBUG_EDIT)
+
+    if (target == gid) {
+	sudo_debug_printf(SUDO_DEBUG_INFO|SUDO_DEBUG_LINENO,
+	    "user gid %u matches directory gid %u", (unsigned int)gid,
+	    (unsigned int)target);
+	debug_return_bool(true);
+    }
+    for (i = 0; i < ngroups; i++) {
+	if (target == groups[i]) {
+	    sudo_debug_printf(SUDO_DEBUG_INFO|SUDO_DEBUG_LINENO,
+		"user gid %u matches directory gid %u", (unsigned int)gid,
+		(unsigned int)target);
+	    debug_return_bool(true);
+	}
+    }
+    debug_return_bool(false);
+}
+
+#ifndef HAVE_OPENAT
+/* This does not support AT_FDCWD... */
+static int
+sudo_openat(int dfd, const char *path, int flags, mode_t mode)
+{
+    int fd, odfd;
+    debug_decl(sudo_openat, SUDO_DEBUG_EDIT)
+
+    /* Save cwd */
+    if ((odfd = open(".", O_RDONLY)) == -1)
+	debug_return_int(-1);
+
+    if (fchdir(dfd) == -1) {
+	close(odfd);
+	debug_return_int(-1);
+    }
+
+    fd = open(path, flags, mode);
+
+    /* Restore cwd */
+    if (fchdir(odfd) == -1)
+	sudo_fatal(_("unable to restore current working directory"));
+    close(odfd);
+
+    debug_return_int(fd);
+}
+#define openat sudo_openat
+#endif /* HAVE_OPENAT */
+
+/*
+ * Returns true if the directory described by sb is writable
+ * by the user.  We treat directories with the sticky bit as
+ * unwritable unless they are owned by the user.
+ */
+static bool
+dir_is_writable(struct stat *sb, uid_t uid, gid_t gid, int ngroups,
+    GETGROUPS_T *groups)
+{
+    debug_decl(dir_is_writable, SUDO_DEBUG_EDIT)
+
+    /* If the user owns the dir we always consider it writable. */
+    if (sb->st_uid == uid) {
+	sudo_debug_printf(SUDO_DEBUG_INFO|SUDO_DEBUG_LINENO,
+	    "user uid %u matches directory uid %u", (unsigned int)uid,
+	    (unsigned int)sb->st_uid);
+	debug_return_bool(true);
+    }
+
+    /* Other writable? */
+    if (ISSET(sb->st_mode, S_IWOTH)) {
+	sudo_debug_printf(SUDO_DEBUG_INFO|SUDO_DEBUG_LINENO,
+	    "directory is writable by other");
+	debug_return_bool(true);
+    }
+
+    /* Group writable? */
+    if (ISSET(sb->st_mode, S_IWGRP)) {
+	if (group_matches(sb->st_gid, gid, ngroups, groups)) {
+	    sudo_debug_printf(SUDO_DEBUG_INFO|SUDO_DEBUG_LINENO,
+		"directory is writable by one of the user's groups");
+	    debug_return_bool(true);
+	}
+    }
+
+    debug_return_bool(false);
+}
+
+static int
+sudo_edit_open_nonwritable(char *path, int oflags, mode_t mode)
+{
+    char *base, *dir;
+    struct stat sb;
+    int dfd, fd;
+    debug_decl(sudo_edit_open_nonwritable, SUDO_DEBUG_EDIT)
+
+    base = strrchr(path, '/');
+    if (base != NULL) {
+	*base++ = '\0';
+	dir = path;
+    } else {
+	base = path;
+	dir = ".";
+    }
+#ifdef O_PATH
+    if ((dfd = open(dir, O_PATH)) != -1) {
+	/* Linux kernels < 3.6 can't do fstat on O_PATH fds. */
+	if (fstat(dfd, &sb) == -1) {
+	    close(dfd);
+	    dfd = open(dir, O_RDONLY);
+	    if (fstat(dfd, &sb) == -1) {
+		close(dfd);
+		dfd = -1;
+	    }
+	}
+    }
+#else
+    if ((dfd = open(dir, O_RDONLY)) != -1) {
+	if (fstat(dfd, &sb) == -1) {
+	    close(dfd);
+	    dfd = -1;
+	}
+    }
+#endif
+    if (base != path)
+	base[-1] = '/';			/* restore path */
+    if (dfd == -1)
+	debug_return_int(-1);
+
+    if (dir_is_writable(&sb, user_details.uid, user_details.gid,
+	user_details.ngroups, user_details.groups)) {
+	close(dfd);
+	errno = ENOTDIR;
+	debug_return_int(-1);
+    }
+
+    fd = openat(dfd, path, oflags, mode);
+    close(dfd);
+    debug_return_int(fd);
+}
+
 #ifdef O_NOFOLLOW
 static int
-sudo_edit_open(const char *path, int oflags, mode_t mode, int sflags)
+sudo_edit_open(char *path, int oflags, mode_t mode, int sflags)
 {
     int fd;
+    debug_decl(sudo_edit_open, SUDO_DEBUG_EDIT)
 
     if (!ISSET(sflags, CD_SUDOEDIT_FOLLOW))
 	oflags |= O_NOFOLLOW;
-    fd = open(path, oflags|O_NONBLOCK, mode);
+    if (ISSET(sflags, CD_SUDOEDIT_CHECKDIR) && user_details.uid != 0)
+	fd = sudo_edit_open_nonwritable(path, oflags|O_NONBLOCK, mode);
+    else
+	fd = open(path, oflags|O_NONBLOCK, mode);
     if (fd != -1 && !ISSET(oflags, O_NONBLOCK))
 	(void) fcntl(fd, F_SETFL, fcntl(fd, F_GETFL, 0) & ~O_NONBLOCK);
-    return fd;
+    debug_return_int(fd);
 }
 #else
 static int
-sudo_edit_open(const char *path, int oflags, mode_t mode, int sflags)
+sudo_edit_open(char *path, int oflags, mode_t mode, int sflags)
 {
     struct stat sb1, sb2;
     int fd;
+    debug_decl(sudo_edit_open, SUDO_DEBUG_EDIT)
 
-    if ((fd = open(path, oflags|O_NONBLOCK, mode)) == -1)
+    if (ISSET(sflags, CD_SUDOEDIT_CHECKDIR) && user_details.uid != 0)
-	return -1;
+	fd = sudo_edit_open_nonwritable(path, oflags|O_NONBLOCK, mode);
+    else
+	fd = open(path, oflags|O_NONBLOCK, mode);
+    if (fd == -1)
+	debug_return_int(-1);
     if (!ISSET(oflags, O_NONBLOCK))
 	(void) fcntl(fd, F_SETFL, fcntl(fd, F_GETFL, 0) & ~O_NONBLOCK);
 
@@ -186,7 +339,7 @@ sudo_edit_open(const char *path, int oflags, mode_t mode, int sflags)
 	const int serrno = errno;
 	close(fd);
 	errno = serrno;
-	return -1;
+	debug_return_int(-1);
     }
 
     /*
@@ -198,11 +351,11 @@ sudo_edit_open(const char *path, int oflags, mode_t mode, int sflags)
 	    sb1.st_dev != sb2.st_dev || sb1.st_ino != sb2.st_ino) {
 	    close(fd);
 	    errno = ELOOP;
-	    return -1;
+	    debug_return_int(-1);
 	}
     }
 
-    return fd;
+    debug_return_int(fd);
 }
 #endif /* O_NOFOLLOW */
 
@@ -214,7 +367,7 @@ sudo_edit_open(const char *path, int oflags, mode_t mode, int sflags)
  */
 static int
 sudo_edit_create_tfiles(struct command_details *command_details,
-    struct tempfile *tf, char * const files[], int nfiles)
+    struct tempfile *tf, char *files[], int nfiles)
 {
     int i, j, tfd, ofd, rc;
     char buf[BUFSIZ];
@@ -252,6 +405,9 @@ sudo_edit_create_tfiles(struct command_details *command_details,
 	    if (ofd == -1 && errno == ELOOP) {
 		sudo_warnx(U_("%s: editing symbolic links is not permitted"),
 		    files[i]);
+	    } else if (ofd == -1 && errno == ENOTDIR) {
+		sudo_warnx(U_("%s: editing files in a writable directory is not permitted"),
+		    files[i]);
 	    } else {
 		sudo_warn("%s", files[i]);
 	    }
@@ -406,7 +562,7 @@ sudo_edit_copy_tfiles(struct command_details *command_details,
 #ifdef HAVE_SELINUX
 static int
 selinux_edit_create_tfiles(struct command_details *command_details,
-    struct tempfile *tf, char * const files[], int nfiles)
+    struct tempfile *tf, char *files[], int nfiles)
 {
     char **sesh_args, **sesh_ap;
     int i, rc, sesh_nargs;