File sudo-CVE-2021-23240.patch of Package sudo.27911
# 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(×[0], ×[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(×[0], ×[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(×[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
