9f5db7
diff -up ./src/copy_file.c.symbolic-link-attack-4 ./src/copy_file.c
9f5db7
--- ./src/copy_file.c.symbolic-link-attack-4	2021-02-02 16:35:18.453036846 +0100
9f5db7
+++ ./src/copy_file.c	2021-02-02 16:38:09.430731749 +0100
9f5db7
@@ -23,6 +23,7 @@
9f5db7
 
9f5db7
 #include <config.h>
9f5db7
 
9f5db7
+#include <sys/stat.h>
9f5db7
 #include <sys/types.h>
9f5db7
 
9f5db7
 #include <stdio.h>
9f5db7
@@ -126,3 +127,35 @@ write_error:
9f5db7
 	debug_return_int(-1);
9f5db7
     }
9f5db7
 }
9f5db7
+
9f5db7
+#ifdef HAVE_SELINUX
9f5db7
+bool
9f5db7
+sudo_check_temp_file(int tfd, const char *tfile, uid_t uid, struct stat *sb)
9f5db7
+{
9f5db7
+    struct stat sbuf;
9f5db7
+    debug_decl(sudo_check_temp_file, SUDO_DEBUG_UTIL);
9f5db7
+
9f5db7
+    if (sb == NULL)
9f5db7
+	sb = &sbu;;
9f5db7
+
9f5db7
+    if (fstat(tfd, sb) == -1) {
9f5db7
+	sudo_warn(U_("unable to stat %s"), tfile);
9f5db7
+	debug_return_bool(false);
9f5db7
+    }
9f5db7
+    if (!S_ISREG(sb->st_mode)) {
9f5db7
+	sudo_warnx(U_("%s: not a regular file"), tfile);
9f5db7
+	debug_return_bool(false);
9f5db7
+    }
9f5db7
+    if ((sb->st_mode & ALLPERMS) != (S_IRUSR|S_IWUSR)) {
9f5db7
+	sudo_warnx(U_("%s: bad file mode: 0%o"), tfile,
9f5db7
+	    (unsigned int)(sb->st_mode & ALLPERMS));
9f5db7
+	debug_return_bool(false);
9f5db7
+    }
9f5db7
+    if (sb->st_uid != uid) {
9f5db7
+	sudo_warnx(U_("%s is owned by uid %u, should be %u"),
9f5db7
+	    tfile, (unsigned int)sb->st_uid, (unsigned int)uid);
9f5db7
+	debug_return_bool(false);
9f5db7
+    }
9f5db7
+    debug_return_bool(true);
9f5db7
+}
9f5db7
+#endif /* SELINUX */
9f5db7
diff -up ./src/sesh.c.symbolic-link-attack-4 ./src/sesh.c
9f5db7
--- ./src/sesh.c.symbolic-link-attack-4	2021-02-02 16:35:18.450036887 +0100
9f5db7
+++ ./src/sesh.c	2021-02-02 16:38:52.907146897 +0100
9f5db7
@@ -134,7 +134,7 @@ main(int argc, char *argv[], char *envp[
9f5db7
 static int
9f5db7
 sesh_sudoedit(int argc, char *argv[])
9f5db7
 {
9f5db7
-    int i, oflags_dst, post, ret = SESH_ERR_FAILURE;
9f5db7
+    int i, oflags_src, oflags_dst, post, ret = SESH_ERR_FAILURE;
9f5db7
     int fd_src = -1, fd_dst = -1, follow = 0;
9f5db7
     ssize_t nread, nwritten;
9f5db7
     struct stat sb;
9f5db7
@@ -178,10 +178,12 @@ sesh_sudoedit(int argc, char *argv[])
9f5db7
 	debug_return_int(SESH_ERR_BAD_PATHS);
9f5db7
 
9f5db7
     /*
9f5db7
-     * Use O_EXCL if we are not in the post editing stage
9f5db7
-     * so that it's ensured that the temporary files are
9f5db7
-     * created by us and that we are not opening any symlinks.
9f5db7
+     * In the pre-editing stage, use O_EXCL to ensure that the temporary
9f5db7
+     * files are created by us and that we are not opening any symlinks.
9f5db7
+     * In the post-editing stage, use O_NOFOLLOW so we don't follow symlinks
9f5db7
+     * when opening the temporary files.
9f5db7
      */
9f5db7
+    oflags_src = O_RDONLY|(post ? O_NONBLOCK|O_NOFOLLOW : follow);
9f5db7
     oflags_dst = O_WRONLY|O_CREAT|(post ? follow : O_EXCL);
9f5db7
     for (i = 0; i < argc - 1; i += 2) {
9f5db7
 	const char *path_src = argv[i];
9f5db7
@@ -191,7 +193,7 @@ sesh_sudoedit(int argc, char *argv[])
9f5db7
 	 * doesn't exist, that's OK, we'll create an empty
9f5db7
 	 * destination file.
9f5db7
 	 */
9f5db7
-	if ((fd_src = open(path_src, O_RDONLY|follow, S_IRUSR|S_IWUSR)) < 0) {
9f5db7
+	if ((fd_src = open(path_src, oflags_src, S_IRUSR|S_IWUSR)) < 0) {
9f5db7
 	    if (errno != ENOENT) {
9f5db7
 		sudo_warn("%s", path_src);
9f5db7
 		if (post) {
9f5db7
@@ -201,6 +203,14 @@ sesh_sudoedit(int argc, char *argv[])
9f5db7
 		    goto cleanup_0;
9f5db7
 	    }
9f5db7
 	}
9f5db7
+	if (post) {
9f5db7
+	    /* Make sure the temporary file is safe and has the proper owner. */
9f5db7
+	    if (!sudo_check_temp_file(fd_src, path_src, geteuid(), &sb)) {
9f5db7
+		ret = SESH_ERR_SOME_FILES;
9f5db7
+		goto nocleanup;
9f5db7
+	    }
9f5db7
+	    fcntl(fd_src, F_SETFL, fcntl(fd_src, F_GETFL, 0) & ~O_NONBLOCK);
9f5db7
+	}
9f5db7
 
9f5db7
 	if ((fd_dst = open(path_dst, oflags_dst, post ?
9f5db7
 	    (S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH) : (S_IRUSR|S_IWUSR))) < 0) {
9f5db7
@@ -218,10 +228,7 @@ sesh_sudoedit(int argc, char *argv[])
9f5db7
 	    off_t len_dst = -1;
9f5db7
 
9f5db7
 	    if (post) {
9f5db7
-		if (fstat(fd_src, &sb) != 0) {
9f5db7
-		    ret = SESH_ERR_SOME_FILES;
9f5db7
-		    goto nocleanup;
9f5db7
-		}
9f5db7
+		/* sudo_check_temp_file() filled in sb for us. */
9f5db7
 		len_src = sb.st_size;
9f5db7
 		if (fstat(fd_dst, &sb) != 0) {
9f5db7
 		    ret = SESH_ERR_SOME_FILES;
9f5db7
diff -up ./src/sudo_edit.c.symbolic-link-attack-4 ./src/sudo_edit.c
9f5db7
--- ./src/sudo_edit.c.symbolic-link-attack-4	2021-02-02 16:35:18.452036860 +0100
9f5db7
+++ ./src/sudo_edit.c	2021-02-02 16:54:25.943429580 +0100
9f5db7
@@ -253,8 +253,10 @@ sudo_edit_mktemp(const char *ofile, char
9f5db7
     } else {
9f5db7
 	len = asprintf(tfile, "%s/%s.XXXXXXXX", edit_tmpdir, cp);
9f5db7
     }
9f5db7
-    if (len == -1)
9f5db7
-	sudo_fatalx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
9f5db7
+    if (len == -1) {
9f5db7
+	sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
9f5db7
+	debug_return_int(-1);
9f5db7
+    }
9f5db7
     tfd = mkstemps(*tfile, suff ? strlen(suff) : 0);
9f5db7
     sudo_debug_printf(SUDO_DEBUG_INFO|SUDO_DEBUG_LINENO,
9f5db7
 	"%s -> %s, fd %d", ofile, *tfile, tfd);
9f5db7
@@ -757,7 +759,8 @@ bad:
9f5db7
 
9f5db7
 #ifdef HAVE_SELINUX
9f5db7
 static int
9f5db7
-selinux_run_helper(char *argv[], char *envp[])
9f5db7
+selinux_run_helper(uid_t uid, gid_t gid, int ngroups, GETGROUPS_T *groups,
9f5db7
+    char *const argv[], char *const envp[])
9f5db7
 {
9f5db7
     int status, ret = SESH_ERR_FAILURE;
9f5db7
     const char *sesh;
9f5db7
@@ -777,8 +780,10 @@ selinux_run_helper(char *argv[], char *e
9f5db7
 	break;
9f5db7
     case 0:
9f5db7
 	/* child runs sesh in new context */
9f5db7
-	if (selinux_setcon() == 0)
9f5db7
+	if (selinux_setcon() == 0) {
9f5db7
+	    switch_user(uid, gid, ngroups, groups);
9f5db7
 	    execve(sesh, argv, envp);
9f5db7
+	}
9f5db7
 	_exit(SESH_ERR_FAILURE);
9f5db7
     default:
9f5db7
 	/* parent waits */
9f5db7
@@ -797,7 +802,7 @@ selinux_edit_create_tfiles(struct comman
9f5db7
     struct tempfile *tf, char *files[], int nfiles)
9f5db7
 {
9f5db7
     char **sesh_args, **sesh_ap;
9f5db7
-    int i, rc, sesh_nargs;
9f5db7
+    int i, rc, error, sesh_nargs, ret = -1;
9f5db7
     struct stat sb;
9f5db7
     debug_decl(selinux_edit_create_tfiles, SUDO_DEBUG_EDIT)
9f5db7
 
9f5db7
@@ -809,7 +814,7 @@ selinux_edit_create_tfiles(struct comman
9f5db7
     sesh_args = sesh_ap = reallocarray(NULL, sesh_nargs, sizeof(char *));
9f5db7
     if (sesh_args == NULL) {
9f5db7
 	sudo_warnx(U_("%s: %s"), __func__, U_("unable to allocate memory"));
9f5db7
-	debug_return_int(-1);
9f5db7
+	goto done;
9f5db7
     }
9f5db7
     *sesh_ap++ = "sesh";
9f5db7
     *sesh_ap++ = "-e";
9f5db7
@@ -817,7 +822,6 @@ selinux_edit_create_tfiles(struct comman
9f5db7
 	*sesh_ap++ = "-h";
9f5db7
     *sesh_ap++ = "0";
9f5db7
 
9f5db7
-    /* XXX - temp files should be created with user's context */
9f5db7
     for (i = 0; i < nfiles; i++) {
9f5db7
 	char *tfile, *ofile = files[i];
9f5db7
 	int tfd;
9f5db7
@@ -835,8 +839,7 @@ selinux_edit_create_tfiles(struct comman
9f5db7
 	if (tfd == -1) {
9f5db7
 	    sudo_warn("mkstemps");
9f5db7
 	    free(tfile);
9f5db7
-	    free(sesh_args);
9f5db7
-	    debug_return_int(-1);
9f5db7
+	    goto done;
9f5db7
 	}
9f5db7
 	/* Helper will re-create temp file with proper security context. */
9f5db7
 	close(tfd);
9f5db7
@@ -847,8 +850,10 @@ selinux_edit_create_tfiles(struct comman
9f5db7
     *sesh_ap = NULL;
9f5db7
 
9f5db7
     /* Run sesh -e [-h] 0 <o1> <t1> ... <on> <tn> */
9f5db7
-    rc = selinux_run_helper(sesh_args, command_details->envp);
9f5db7
-    switch (rc) {
9f5db7
+    error = selinux_run_helper(command_details->uid, command_details->gid,
9f5db7
+	command_details->ngroups, command_details->groups, sesh_args,
9f5db7
+	command_details->envp);
9f5db7
+    switch (error) {
9f5db7
     case SESH_SUCCESS:
9f5db7
 	break;
9f5db7
     case SESH_ERR_BAD_PATHS:
9f5db7
@@ -858,21 +863,34 @@ selinux_edit_create_tfiles(struct comman
9f5db7
     case SESH_ERR_KILLED:
9f5db7
 	sudo_fatalx(U_("sesh: killed by a signal"));
9f5db7
     default:
9f5db7
-	sudo_fatalx(U_("sesh: unknown error %d"), rc);
9f5db7
+	sudo_fatalx(U_("sesh: unknown error %d"), error);
9f5db7
+    goto done;
9f5db7
     }
9f5db7
 
9f5db7
-    /* Chown to user's UID so they can edit the temporary files. */
9f5db7
     for (i = 0; i < nfiles; i++) {
9f5db7
-	if (chown(tf[i].tfile, user_details.uid, user_details.gid) != 0) {
9f5db7
-	    sudo_warn("unable to chown(%s) to %d:%d for editing",
9f5db7
-		tf[i].tfile, user_details.uid, user_details.gid);
9f5db7
-	}
9f5db7
+        int tfd = open(tf[i].tfile, O_RDONLY|O_NONBLOCK|O_NOFOLLOW);
9f5db7
+        if (tfd == -1) {
9f5db7
+            sudo_warn(U_("unable to open %s"), tf[i].tfile);
9f5db7
+            goto done;
9f5db7
+        }
9f5db7
+        if (!sudo_check_temp_file(tfd, tf[i].tfile, command_details->uid, NULL)) {
9f5db7
+            close(tfd);
9f5db7
+            goto done;
9f5db7
+        }
9f5db7
+        if (fchown(tfd, user_details.uid, user_details.gid) != 0) {
9f5db7
+            sudo_warn("unable to chown(%s) to %d:%d for editing",
9f5db7
+                      tf[i].tfile, user_details.uid, user_details.gid);
9f5db7
+            close(tfd);
9f5db7
+            goto done;
9f5db7
+        }
9f5db7
+        close(tfd);
9f5db7
     }
9f5db7
 
9f5db7
+done:
9f5db7
     /* Contents of tf will be freed by caller. */
9f5db7
     free(sesh_args);
9f5db7
 
9f5db7
-    return (nfiles);
9f5db7
+    debug_return_int(ret);
9f5db7
 }
9f5db7
 
9f5db7
 static int
9f5db7
@@ -880,7 +898,8 @@ selinux_edit_copy_tfiles(struct command_
9f5db7
     struct tempfile *tf, int nfiles, struct timespec *times)
9f5db7
 {
9f5db7
     char **sesh_args, **sesh_ap;
9f5db7
-    int i, rc, sesh_nargs, ret = 1;
9f5db7
+    int i, rc, error, sesh_nargs, ret = 1;
9f5db7
+    int tfd = -1;
9f5db7
     struct timespec ts;
9f5db7
     struct stat sb;
9f5db7
     debug_decl(selinux_edit_copy_tfiles, SUDO_DEBUG_EDIT)
9f5db7
@@ -901,33 +920,43 @@ selinux_edit_copy_tfiles(struct command_
9f5db7
 
9f5db7
     /* Construct args for sesh -e 1 */
9f5db7
     for (i = 0; i < nfiles; i++) {
9f5db7
-	if (stat(tf[i].tfile, &sb) == 0) {
9f5db7
-	    mtim_get(&sb, ts);
9f5db7
-	    if (tf[i].osize == sb.st_size && sudo_timespeccmp(&tf[i].omtim, &ts, ==)) {
9f5db7
-		/*
9f5db7
-		 * If mtime and size match but the user spent no measurable
9f5db7
-		 * time in the editor we can't tell if the file was changed.
9f5db7
-		 */
9f5db7
-		if (sudo_timespeccmp(&times[0], &times[1], !=)) {
9f5db7
-		    sudo_warnx(U_("%s unchanged"), tf[i].ofile);
9f5db7
-		    unlink(tf[i].tfile);
9f5db7
-		    continue;
9f5db7
-		}
9f5db7
+	if (tfd != -1)
9f5db7
+	    close(tfd);
9f5db7
+	if ((tfd = open(tf[i].tfile, O_RDONLY|O_NONBLOCK|O_NOFOLLOW)) == -1) {
9f5db7
+	    sudo_warn(U_("unable to open %s"), tf[i].tfile);
9f5db7
+	    continue;
9f5db7
+	}
9f5db7
+	if (!sudo_check_temp_file(tfd, tf[i].tfile, user_details.uid, &sb))
9f5db7
+	    continue;
9f5db7
+	mtim_get(&sb, ts);
9f5db7
+	if (tf[i].osize == sb.st_size && sudo_timespeccmp(&tf[i].omtim, &ts, ==)) {
9f5db7
+	    /*
9f5db7
+	     * If mtime and size match but the user spent no measurable
9f5db7
+	     * time in the editor we can't tell if the file was changed.
9f5db7
+	     */
9f5db7
+	    if (sudo_timespeccmp(&times[0], &times[1], !=)) {
9f5db7
+		sudo_warnx(U_("%s unchanged"), tf[i].ofile);
9f5db7
+		unlink(tf[i].tfile);
9f5db7
+		continue;
9f5db7
 	    }
9f5db7
 	}
9f5db7
 	*sesh_ap++ = tf[i].tfile;
9f5db7
 	*sesh_ap++ = tf[i].ofile;
9f5db7
-	if (chown(tf[i].tfile, command_details->uid, command_details->gid) != 0) {
9f5db7
+	if (fchown(tfd, command_details->uid, command_details->gid) != 0) {
9f5db7
 	    sudo_warn("unable to chown(%s) back to %d:%d", tf[i].tfile,
9f5db7
 		command_details->uid, command_details->gid);
9f5db7
 	}
9f5db7
     }
9f5db7
     *sesh_ap = NULL;
9f5db7
+    if (tfd != -1)
9f5db7
+	close(tfd);
9f5db7
 
9f5db7
     if (sesh_ap - sesh_args > 3) {
9f5db7
 	/* Run sesh -e 1 <t1> <o1> ... <tn> <on> */
9f5db7
-	rc = selinux_run_helper(sesh_args, command_details->envp);
9f5db7
-	switch (rc) {
9f5db7
+	error = selinux_run_helper(command_details->uid, command_details->gid,
9f5db7
+	    command_details->ngroups, command_details->groups, sesh_args,
9f5db7
+	    command_details->envp);
9f5db7
+	switch (error) {
9f5db7
 	case SESH_SUCCESS:
9f5db7
 	    ret = 0;
9f5db7
 	    break;
9f5db7
@@ -941,7 +970,7 @@ selinux_edit_copy_tfiles(struct command_
9f5db7
 	    sudo_warnx(U_("sesh: killed by a signal"));
9f5db7
 	    break;
9f5db7
 	default:
9f5db7
-	    sudo_warnx(U_("sesh: unknown error %d"), rc);
9f5db7
+	    sudo_warnx(U_("sesh: unknown error %d"), error);
9f5db7
 	    break;
9f5db7
 	}
9f5db7
 	if (ret != 0)
9f5db7
@@ -963,7 +992,7 @@ sudo_edit(struct command_details *comman
9f5db7
 {
9f5db7
     struct command_details saved_command_details;
9f5db7
     char **nargv = NULL, **ap, **files = NULL;
9f5db7
-    int errors, i, ac, nargc, rc;
9f5db7
+    int errors, i, ac, nargc, ret;
9f5db7
     int editor_argc = 0, nfiles = 0;
9f5db7
     struct timespec times[2];
9f5db7
     struct tempfile *tf = NULL;
9f5db7
@@ -1058,7 +1087,7 @@ sudo_edit(struct command_details *comman
9f5db7
     command_details->ngroups = user_details.ngroups;
9f5db7
     command_details->groups = user_details.groups;
9f5db7
     command_details->argv = nargv;
9f5db7
-    rc = run_command(command_details);
9f5db7
+    ret = run_command(command_details);
9f5db7
     if (sudo_gettime_real(&times[1]) == -1) {
9f5db7
 	sudo_warn(U_("unable to read the clock"));
9f5db7
 	goto cleanup;
9f5db7
@@ -1080,14 +1109,16 @@ sudo_edit(struct command_details *comman
9f5db7
     else
9f5db7
 #endif
9f5db7
 	errors = sudo_edit_copy_tfiles(command_details, tf, nfiles, times);
9f5db7
-    if (errors)
9f5db7
-	goto cleanup;
9f5db7
+    if (errors) {
9f5db7
+        /* Preserve the edited temporary files. */
9f5db7
+        ret = W_EXITCODE(1, 0);
9f5db7
+    }
9f5db7
 
9f5db7
     for (i = 0; i < nfiles; i++)
9f5db7
 	free(tf[i].tfile);
9f5db7
     free(tf);
9f5db7
     free(nargv);
9f5db7
-    debug_return_int(rc);
9f5db7
+    debug_return_int(ret);
9f5db7
 
9f5db7
 cleanup:
9f5db7
     /* Clean up temp files and return. */
9f5db7
diff -up ./src/sudo_exec.h.symbolic-link-attack-4 ./src/sudo_exec.h
9f5db7
--- ./src/sudo_exec.h.symbolic-link-attack-4	2021-02-02 16:35:18.452036860 +0100
9f5db7
+++ ./src/sudo_exec.h	2021-02-02 16:35:18.454036833 +0100
9f5db7
@@ -1,7 +1,7 @@
9f5db7
 /*
9f5db7
  * SPDX-License-Identifier: ISC
9f5db7
  *
9f5db7
- * Copyright (c) 2010-2016 Todd C. Miller <Todd.Miller@sudo.ws>
9f5db7
+ * Copyright (c) 2010-2017, 2020-2021 Todd C. Miller <Todd.Miller@sudo.ws>
9f5db7
  *
9f5db7
  * Permission to use, copy, modify, and distribute this software for any
9f5db7
  * purpose with or without fee is hereby granted, provided that the above
9f5db7
@@ -84,9 +84,11 @@
9f5db7
  */
9f5db7
 struct command_details;
9f5db7
 struct command_status;
9f5db7
+struct stat;
9f5db7
 
9f5db7
 /* copy_file.c */
9f5db7
 int sudo_copy_file(const char *src, int src_fd, off_t src_len, const char *dst, int dst_fd, off_t dst_len);
9f5db7
+bool sudo_check_temp_file(int tfd, const char *tname, uid_t uid, struct stat *sb);
9f5db7
 
9f5db7
 /* exec.c */
9f5db7
 void exec_cmnd(struct command_details *details, int errfd);