File sudo-CVE-2021-23240.patch of Package sudo

Overview Repositories Revisions Requests Users Attributes Meta

File sudo-CVE-2021-23240.patch of Package sudo

# HG changeset patch
# User Todd C. Miller <Todd.Miller@sudo.ws>
# Date 1609953360 25200
# Node ID 8fcb36ef422a251fe33738a347551439944a4a37
# Parent  ea19d0073c02951bbbf35342dd63304da83edce8
Add security checks before using temp files for SELinux RBAC sudoedit.
Otherwise, it may be possible for the user running sudoedit to
replace the newly-created temporary files with a symbolic link and
have sudoedit set the owner of an arbitrary file.
Problem reported by Matthias Gerstner of SUSE.

diff --git a/src/Makefile.in b/src/Makefile.in
index 099193c..88a7bb7 100644
--- a/src/Makefile.in
+++ b/src/Makefile.in
@@ -118,7 +118,7 @@ SHELL = @SHELL@
 
 PROGS = @PROGS@
 
-OBJS = conversation.o env_hooks.o exec.o exec_common.o exec_monitor.o \
+OBJS = conversation.o copy_file.o env_hooks.o exec.o exec_common.o exec_monitor.o \
        exec_nopty.o exec_pty.o get_pty.o hooks.o net_ifs.o load_plugins.o \
        parse_args.o preserve_fds.o signal.o sudo.o sudo_edit.o \
        tcsetpgrp_nobg.o tgetpass.o ttyname.o utmp.o @SUDO_OBJS@
@@ -127,7 +127,7 @@ IOBJS = $(OBJS:.o=.i) sesh.i
 
 POBJS = $(IOBJS:.i=.plog)
 
-SESH_OBJS = sesh.o exec_common.o
+SESH_OBJS = sesh.o copy_file.o exec_common.o
 
 CHECK_NOEXEC_OBJS = check_noexec.o exec_common.o
 
@@ -326,6 +326,24 @@ conversation.i: $(srcdir)/conversation.c $(incdir)/compat/stdbool.h \
 	$(CC) -E -o $@ $(CPPFLAGS) $<
 conversation.plog: conversation.i
 	rm -f $@; pvs-studio --cfg $(PVS_CFG) --sourcetree-root $(top_srcdir) --skip-cl-exe yes --source-file $(srcdir)/conversation.c --i-file $< --output-file $@
+copy_file.o: $(srcdir)/copy_file.c $(incdir)/compat/stdbool.h \
+             $(incdir)/sudo_compat.h $(incdir)/sudo_conf.h \
+             $(incdir)/sudo_debug.h $(incdir)/sudo_event.h \
+             $(incdir)/sudo_fatal.h $(incdir)/sudo_gettext.h \
+             $(incdir)/sudo_plugin.h $(incdir)/sudo_queue.h \
+             $(incdir)/sudo_util.h $(srcdir)/sudo.h $(top_builddir)/config.h \
+             $(top_builddir)/pathnames.h
+	$(CC) -c $(CPPFLAGS) $(CFLAGS) $(ASAN_CFLAGS) $(PIE_CFLAGS) $(SSP_CFLAGS) $(srcdir)/copy_file.c
+copy_file.i: $(srcdir)/copy_file.c $(incdir)/compat/stdbool.h \
+             $(incdir)/sudo_compat.h $(incdir)/sudo_conf.h \
+             $(incdir)/sudo_debug.h $(incdir)/sudo_event.h \
+             $(incdir)/sudo_fatal.h $(incdir)/sudo_gettext.h \
+             $(incdir)/sudo_plugin.h $(incdir)/sudo_queue.h \
+             $(incdir)/sudo_util.h $(srcdir)/sudo.h $(top_builddir)/config.h \
+             $(top_builddir)/pathnames.h
+	$(CC) -E -o $@ $(CPPFLAGS) $<
+copy_file.plog: copy_file.i
+	rm -f $@; pvs-studio --cfg $(PVS_CFG) --sourcetree-root $(top_srcdir) --skip-cl-exe yes --source-file $(srcdir)/copy_file.c --i-file $< --output-file $@
 env_hooks.o: $(srcdir)/env_hooks.c $(incdir)/compat/stdbool.h \
              $(incdir)/sudo_compat.h $(incdir)/sudo_conf.h \
              $(incdir)/sudo_debug.h $(incdir)/sudo_dso.h \
@@ -579,7 +597,7 @@ selinux.plog: selinux.i
 sesh.o: $(srcdir)/sesh.c $(incdir)/compat/stdbool.h $(incdir)/sudo_compat.h \
         $(incdir)/sudo_conf.h $(incdir)/sudo_debug.h $(incdir)/sudo_fatal.h \
         $(incdir)/sudo_gettext.h $(incdir)/sudo_plugin.h \
-        $(incdir)/sudo_queue.h $(incdir)/sudo_util.h $(srcdir)/sudo_exec.h \
+        $(incdir)/sudo_queue.h $(incdir)/sudo_util.h $(srcdir)/sudo_exec.h $(srcdir)/sudo_edit.h \
         $(top_builddir)/config.h
 	$(CC) -c $(CPPFLAGS) $(CFLAGS) $(ASAN_CFLAGS) $(PIE_CFLAGS) $(SSP_CFLAGS) $(srcdir)/sesh.c
 sesh.i: $(srcdir)/sesh.c $(incdir)/compat/stdbool.h $(incdir)/sudo_compat.h \
@@ -642,7 +660,7 @@ sudo_edit.o: $(srcdir)/sudo_edit.c $(incdir)/compat/stdbool.h \
              $(incdir)/sudo_compat.h $(incdir)/sudo_conf.h \
              $(incdir)/sudo_debug.h $(incdir)/sudo_fatal.h \
              $(incdir)/sudo_gettext.h $(incdir)/sudo_queue.h \
-             $(incdir)/sudo_util.h $(srcdir)/sudo.h $(srcdir)/sudo_exec.h \
+             $(incdir)/sudo_util.h $(srcdir)/sudo.h $(srcdir)/sudo_exec.h $(srcdir)/sudo_edit.h \
              $(top_builddir)/config.h $(top_builddir)/pathnames.h
 	$(CC) -c $(CPPFLAGS) $(CFLAGS) $(ASAN_CFLAGS) $(PIE_CFLAGS) $(SSP_CFLAGS) $(srcdir)/sudo_edit.c
 sudo_edit.i: $(srcdir)/sudo_edit.c $(incdir)/compat/stdbool.h \
diff --git a/src/sesh.c b/src/sesh.c
index 873748e..79565f3 100644
--- a/src/sesh.c
+++ b/src/sesh.c
@@ -41,6 +41,7 @@
 #include "sudo_gettext.h"	/* must be included before sudo_compat.h */
 
 #include "sudo_compat.h"
+#include "sudo_edit.h"
 #include "sudo_fatal.h"
 #include "sudo_conf.h"
 #include "sudo_debug.h"
@@ -132,7 +133,7 @@ main(int argc, char *argv[], char *envp[])
 static int
 sesh_sudoedit(int argc, char *argv[])
 {
-    int i, oflags_dst, post, ret = SESH_ERR_FAILURE;
+    int i, oflags_src, oflags_dst, post, ret = SESH_ERR_FAILURE;
     int fd_src = -1, fd_dst = -1, follow = 0;
     ssize_t nread, nwritten;
     struct stat sb;
@@ -176,10 +177,12 @@ sesh_sudoedit(int argc, char *argv[])
 	debug_return_int(SESH_ERR_BAD_PATHS);
 
     /*
-     * Use O_EXCL if we are not in the post editing stage
-     * so that it's ensured that the temporary files are
-     * created by us and that we are not opening any symlinks.
+     * In the pre-editing stage, use O_EXCL to ensure that the temporary
+     * files are created by us and that we are not opening any symlinks.
+     * In the post-editing stage, use O_NOFOLLOW so we don't follow symlinks
+     * when opening the temporary files.
      */
+    oflags_src = O_RDONLY|(post ? O_NONBLOCK|O_NOFOLLOW : follow);
     oflags_dst = O_WRONLY|O_TRUNC|O_CREAT|(post ? follow : O_EXCL);
     for (i = 0; i < argc - 1; i += 2) {
 	const char *path_src = argv[i];
@@ -189,7 +192,7 @@ sesh_sudoedit(int argc, char *argv[])
 	 * doesn't exist, that's OK, we'll create an empty
 	 * destination file.
 	 */
-	if ((fd_src = open(path_src, O_RDONLY|follow, S_IRUSR|S_IWUSR)) < 0) {
+	if ((fd_src = open(path_src, oflags_src, S_IRUSR|S_IWUSR)) < 0) {
 	    if (errno != ENOENT) {
 		sudo_warn("%s", path_src);
 		if (post) {
@@ -199,6 +202,14 @@ sesh_sudoedit(int argc, char *argv[])
 		    goto cleanup_0;
 	    }
 	}
+	if (post) {
+	    /* Make sure the temporary file is safe and has the proper owner. */
+	    if (!sudo_check_temp_file(fd_src, path_src, geteuid(), &sb)) {
+		ret = SESH_ERR_SOME_FILES;
+		goto nocleanup;
+	    }
+	    fcntl(fd_src, F_SETFL, fcntl(fd_src, F_GETFL, 0) & ~O_NONBLOCK);
+	}
 
 	if ((fd_dst = open(path_dst, oflags_dst, post ?
 	    (S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH) : (S_IRUSR|S_IWUSR))) < 0) {
diff --git a/src/sudo_edit.c b/src/sudo_edit.c
index 888b277..679c0b0 100644
--- a/src/sudo_edit.c
+++ b/src/sudo_edit.c
@@ -252,8 +252,10 @@ sudo_edit_mktemp(const char *ofile, char **tfile)
     } else {
 	len = asprintf(tfile, "%s/%s.XXXXXXXX", edit_tmpdir, cp);
     }
-    if (len == -1)
-	sudo_fatalx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
+    if (len == -1) {
+	sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
+	debug_return_int(-1);
+    }
     tfd = mkstemps(*tfile, suff ? strlen(suff) : 0);
     sudo_debug_printf(SUDO_DEBUG_INFO|SUDO_DEBUG_LINENO,
 	"%s -> %s, fd %d", ofile, *tfile, tfd);
@@ -784,11 +786,11 @@ selinux_edit_create_tfiles(struct command_details *command_details,
     struct tempfile *tf, char *files[], int nfiles)
 {
     char **sesh_args, **sesh_ap;
-    int i, rc, sesh_nargs;
+    int i, error, sesh_nargs, ret = -1;
     struct stat sb;
     struct command_details saved_command_details;
     debug_decl(selinux_edit_create_tfiles, SUDO_DEBUG_EDIT)
-    
+
     /* Prepare selinux stuff (setexeccon) */
     if (selinux_setup(command_details->selinux_role,
 	command_details->selinux_type, NULL, -1) != 0)
@@ -801,12 +803,12 @@ selinux_edit_create_tfiles(struct command_details *command_details,
     memcpy(&saved_command_details, command_details, sizeof(struct command_details));
     command_details->command = _PATH_SUDO_SESH;
     command_details->flags |= CD_SUDOEDIT_COPY;
-    
+
     sesh_nargs = 4 + (nfiles * 2) + 1;
     sesh_args = sesh_ap = reallocarray(NULL, sesh_nargs, sizeof(char *));
     if (sesh_args == NULL) {
 	sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
-	debug_return_int(-1);
+	goto done;
     }
     *sesh_ap++ = "sesh";
     *sesh_ap++ = "-e";
@@ -831,8 +833,7 @@ selinux_edit_create_tfiles(struct command_details *command_details,
 	if (tfd == -1) {
 	    sudo_warn("mkstemps");
 	    free(tfile);
-	    free(sesh_args);
-	    debug_return_int(-1);
+	    goto done;
 	}
 	/* Helper will re-create temp file with proper security context. */
 	close(tfd);
@@ -844,8 +845,8 @@ selinux_edit_create_tfiles(struct command_details *command_details,
 
     /* Run sesh -e [-h] 0 <o1> <t1> ... <on> <tn> */
     command_details->argv = sesh_args;
-    rc = run_command(command_details);
-    switch (rc) {
+    error = run_command(command_details);
+    switch (error) {
     case SESH_SUCCESS:
 	break;
     case SESH_ERR_BAD_PATHS:
@@ -853,26 +854,40 @@ selinux_edit_create_tfiles(struct command_details *command_details,
     case SESH_ERR_NO_FILES:
 	sudo_fatalx(U_("sesh: unable to create temporary files"));
     default:
-	sudo_fatalx(U_("sesh: unknown error %d"), rc);
+	sudo_fatalx(U_("sesh: unknown error %d"), error);
     }
 
     /* Restore saved command_details. */
     command_details->command = saved_command_details.command;
     command_details->flags = saved_command_details.flags;
     command_details->argv = saved_command_details.argv;
-    
-    /* Chown to user's UID so they can edit the temporary files. */
+
     for (i = 0; i < nfiles; i++) {
-	if (chown(tf[i].tfile, user_details.uid, user_details.gid) != 0) {
-	    sudo_warn("unable to chown(%s) to %d:%d for editing",
-		tf[i].tfile, user_details.uid, user_details.gid);
-	}
+      int tfd = open(tf[i].tfile, O_RDONLY|O_NONBLOCK|O_NOFOLLOW);
+      if (tfd == -1) {
+          sudo_warn(U_("unable to open %s"), tf[i].tfile);
+          goto done;
+      }
+      if (!sudo_check_temp_file(tfd, tf[i].tfile, command_details->uid, NULL)) {
+          close(tfd);
+          goto done;
+      }
+      if (fchown(tfd, user_details.uid, user_details.gid) != 0) {
+          sudo_warn("unable to chown(%s) to %d:%d for editing",
+       		tf[i].tfile, user_details.uid, user_details.gid);
+          close(tfd);
+          goto done;
+      }
+      close(tfd);
     }
+    ret = nfiles;
+
+    done:
 
     /* Contents of tf will be freed by caller. */
     free(sesh_args);
 
-    return (nfiles);
+    debug_return_int(ret);
 }
 
 static int
@@ -880,12 +895,13 @@ selinux_edit_copy_tfiles(struct command_details *command_details,
     struct tempfile *tf, int nfiles, struct timespec *times)
 {
     char **sesh_args, **sesh_ap;
-    int i, rc, sesh_nargs, ret = 1;
+    int i, error, sesh_nargs, ret = 1;
+    int tfd = -1;
     struct command_details saved_command_details;
     struct timespec ts;
     struct stat sb;
     debug_decl(selinux_edit_copy_tfiles, SUDO_DEBUG_EDIT)
-    
+
     /* Prepare selinux stuff (setexeccon) */
     if (selinux_setup(command_details->selinux_role,
 	command_details->selinux_type, NULL, -1) != 0)
@@ -898,7 +914,7 @@ selinux_edit_copy_tfiles(struct command_details *command_details,
     memcpy(&saved_command_details, command_details, sizeof(struct command_details));
     command_details->command = _PATH_SUDO_SESH;
     command_details->flags |= CD_SUDOEDIT_COPY;
-    
+
     sesh_nargs = 3 + (nfiles * 2) + 1;
     sesh_args = sesh_ap = reallocarray(NULL, sesh_nargs, sizeof(char *));
     if (sesh_args == NULL) {
@@ -911,34 +927,42 @@ selinux_edit_copy_tfiles(struct command_details *command_details,
 
     /* Construct args for sesh -e 1 */
     for (i = 0; i < nfiles; i++) {
-	if (stat(tf[i].tfile, &sb) == 0) {
-	    mtim_get(&sb, ts);
-	    if (tf[i].osize == sb.st_size && sudo_timespeccmp(&tf[i].omtim, &ts, ==)) {
-		/*
-		 * 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_timespeccmp(&times[0], &times[1], !=)) {
-		    sudo_warnx(U_("%s unchanged"), tf[i].ofile);
-		    unlink(tf[i].tfile);
-		    continue;
-		}
+      if (tfd != -1)
+          close(tfd);
+      if ((tfd = open(tf[i].tfile, O_RDONLY|O_NONBLOCK|O_NOFOLLOW)) == -1) {
+          sudo_warn(U_("unable to open %s"), tf[i].tfile);
+          continue;
+      }
+      if (!sudo_check_temp_file(tfd, tf[i].tfile, user_details.uid, &sb))
+          continue;
+      mtim_get(&sb, ts);
+      if (tf[i].osize == sb.st_size && sudo_timespeccmp(&tf[i].omtim, &ts, ==)) {
+          /*
+           * 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_timespeccmp(&times[0], &times[1], !=)) {
+      	sudo_warnx(U_("%s unchanged"), tf[i].ofile);
+      	unlink(tf[i].tfile);
+      	continue;
 	    }
 	}
 	*sesh_ap++ = tf[i].tfile;
 	*sesh_ap++ = tf[i].ofile;
-	if (chown(tf[i].tfile, command_details->uid, command_details->gid) != 0) {
+	if (fchown(tfd, command_details->uid, command_details->gid) != 0) {
 	    sudo_warn("unable to chown(%s) back to %d:%d", tf[i].tfile,
 		command_details->uid, command_details->gid);
 	}
     }
     *sesh_ap = NULL;
+    if (tfd != -1)
+    	close(tfd);
 
     if (sesh_ap - sesh_args > 3) {
 	/* Run sesh -e 1 <t1> <o1> ... <tn> <on> */
 	command_details->argv = sesh_args;
-	rc = run_command(command_details);
-	switch (rc) {
+	error = run_command(command_details);
+	switch (error) {
 	case SESH_SUCCESS:
 	    ret = 0;
 	    break;
@@ -951,7 +975,7 @@ selinux_edit_copy_tfiles(struct command_details *command_details,
 	    sudo_warnx(U_("contents of edit session left in %s"), edit_tmpdir);
 	    break;
 	default:
-	    sudo_warnx(U_("sesh: unknown error %d"), rc);
+	    sudo_warnx(U_("sesh: unknown error %d"), error);
 	    break;
 	}
     }
@@ -976,7 +1000,7 @@ sudo_edit(struct command_details *command_details)
 {
     struct command_details saved_command_details;
     char **nargv = NULL, **ap, **files = NULL;
-    int errors, i, ac, nargc, rc;
+    int errors, i, ac, nargc, ret;
     int editor_argc = 0, nfiles = 0;
     struct timespec times[2];
     struct tempfile *tf = NULL;
@@ -1022,7 +1046,7 @@ sudo_edit(struct command_details *command_details)
 #ifdef HAVE_SELINUX
     if (ISSET(command_details->flags, CD_RBAC_ENABLED))
 	nfiles = selinux_edit_create_tfiles(command_details, tf, files, nfiles);
-    else 
+    else
 #endif
 	nfiles = sudo_edit_create_tfiles(command_details, tf, files, nfiles);
     if (nfiles <= 0)
@@ -1061,7 +1085,7 @@ sudo_edit(struct command_details *command_details)
     command_details->ngroups = user_details.ngroups;
     command_details->groups = user_details.groups;
     command_details->argv = nargv;
-    rc = run_command(command_details);
+    ret = run_command(command_details);
     if (sudo_gettime_real(&times[1]) == -1) {
 	sudo_warn(U_("unable to read the clock"));
 	goto cleanup;
@@ -1090,7 +1114,7 @@ sudo_edit(struct command_details *command_details)
 	free(tf[i].tfile);
     free(tf);
     free(nargv);
-    debug_return_int(rc);
+    debug_return_int(errors);
 
 cleanup:
     /* Clean up temp files and return. */
diff --git a/src/sudo_exec.h b/src/sudo_exec.h
index c8c7941..91a2e8c 100644
--- a/src/sudo_exec.h
+++ b/src/sudo_exec.h
@@ -81,6 +81,7 @@
  */
 struct command_details;
 struct command_status;
+struct stat;
 
 /* exec.c */
 void exec_cmnd(struct command_details *details, int errfd);
Index: sudo-1.8.22/src/copy_file.c
===================================================================
--- /dev/null
+++ sudo-1.8.22/src/copy_file.c
@@ -0,0 +1,40 @@
+#include <config.h>
+
+#include <sys/stat.h>
+
+#include <stdlib.h>
+#include <unistd.h>
+#include <errno.h>
+
+#include "sudo.h"
+
+#ifdef HAVE_SELINUX
+bool
+sudo_check_temp_file(int tfd, const char *tfile, uid_t uid, struct stat *sb)
+{
+    struct stat sbuf;
+    debug_decl(sudo_check_temp_file, SUDO_DEBUG_UTIL);
+
+    if (sb == NULL)
+       sb = &sbuf;
+
+    if (fstat(tfd, sb) == -1) {
+       sudo_warn(U_("unable to stat %s"), tfile);
+       debug_return_bool(false);
+    }
+    if (!S_ISREG(sb->st_mode)) {
+       sudo_warnx(U_("%s: not a regular file"), tfile);
+       debug_return_bool(false);
+    }
+    if ((sb->st_mode & ALLPERMS) != (S_IRUSR|S_IWUSR)) {
+       sudo_warnx(U_("%s: bad file mode: 0%o"), tfile, sb->st_mode & ALLPERMS);
+       debug_return_bool(false);
+    }
+    if (sb->st_uid != uid) {
+       sudo_warnx(U_("%s is owned by uid %u, should be %u"),
+           tfile, (unsigned int)sb->st_uid, (unsigned int)uid);
+       debug_return_bool(false);
+    }
+    debug_return_bool(true);
+}
+#endif /* SELINUX */
Index: sudo-1.8.22/src/sudo_edit.h
===================================================================
--- /dev/null
+++ sudo-1.8.22/src/sudo_edit.h
@@ -0,0 +1,7 @@
+
+#ifndef SUDO_EDIT_H
+#define SUDO_EDIT_H
+
+bool sudo_check_temp_file(int tfd, const char *tname, uid_t uid, struct stat *sb);
+
+#endif

Places

File sudo-CVE-2021-23240.patch of Package sudo

Places