Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
SUSE:SLE-15-SP7:GA
sudo.25988
sudo-CVE-2021-23240.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File sudo-CVE-2021-23240.patch of Package sudo.25988
# 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
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
