diff -up ./src/copy_file.c.symbolic-link-attack-2 ./src/copy_file.c --- ./src/copy_file.c.symbolic-link-attack-2 2021-02-02 15:31:20.555340446 +0100 +++ ./src/copy_file.c 2021-02-02 15:31:20.555340446 +0100 @@ -0,0 +1,128 @@ +/* + * SPDX-License-Identifier: ISC + * + * Copyright (c) 2020 Todd C. Miller + * + * Permission to use, copy, modify, and distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ + +/* + * This is an open source non-commercial project. Dear PVS-Studio, please check it. + * PVS-Studio Static Code Analyzer for C, C++ and C#: http://www.viva64.com + */ + +#include + +#include + +#include +#include +#include +#include + +#include "sudo.h" + +/* + * Extend the given fd to the specified size in bytes. + * We do this to allocate disk space up-front before overwriting + * the original file with the temporary. Otherwise, we could + * we run out of disk space after truncating the original file. + */ +static int +sudo_extend_file(int fd, const char *name, off_t new_size) +{ + off_t old_size, size; + ssize_t nwritten; + char zeroes[BUFSIZ] = { '\0' }; + debug_decl(sudo_extend_file, SUDO_DEBUG_UTIL); + + if ((old_size = lseek(fd, 0, SEEK_END)) == -1) { + sudo_warn("lseek"); + debug_return_int(-1); + } + sudo_debug_printf(SUDO_DEBUG_INFO, "%s: extending %s from %lld to %lld", + __func__, name, (long long)old_size, (long long)new_size); + + for (size = old_size; size < new_size; size += nwritten) { + size_t len = new_size - size; + if (len > sizeof(zeroes)) + len = sizeof(zeroes); + nwritten = write(fd, zeroes, len); + if (nwritten == -1) { + int serrno = errno; + if (ftruncate(fd, old_size) == -1) { + sudo_debug_printf( + SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO|SUDO_DEBUG_ERRNO, + "unable to truncate %s to %lld", name, (long long)old_size); + } + errno = serrno; + debug_return_int(-1); + } + } + if (lseek(fd, 0, SEEK_SET) == -1) { + sudo_warn("lseek"); + debug_return_int(-1); + } + + debug_return_int(0); +} + +/* + * Copy the contents of src_fd into dst_fd. + * Returns 0 on success or -1 on error. + */ +int +sudo_copy_file(const char *src, int src_fd, off_t src_len, const char *dst, + int dst_fd, off_t dst_len) +{ + char buf[BUFSIZ]; + ssize_t nwritten, nread; + debug_decl(sudo_copy_file, SUDO_DEBUG_UTIL); + + /* Extend the file to the new size if larger before copying. */ + if (dst_len > 0 && src_len > dst_len) { + if (sudo_extend_file(dst_fd, dst, src_len) == -1) + goto write_error; + } + + /* Overwrite the old file with the new contents. */ + while ((nread = read(src_fd, buf, sizeof(buf))) > 0) { + ssize_t off = 0; + do { + nwritten = write(dst_fd, buf + off, nread - off); + if (nwritten == -1) + goto write_error; + off += nwritten; + } while (nread > off); + } + if (nread == 0) { + /* success, read to EOF */ + if (src_len < dst_len) { + /* We don't open with O_TRUNC so must truncate manually. */ + if (ftruncate(dst_fd, src_len) == -1) { + sudo_debug_printf( + SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO|SUDO_DEBUG_ERRNO, + "unable to truncate %s to %lld", dst, (long long)src_len); + goto write_error; + } + } + debug_return_int(0); + } else if (nread < 0) { + sudo_warn(U_("unable to read from %s"), src); + debug_return_int(-1); + } else { +write_error: + sudo_warn(U_("unable to write to %s"), dst); + debug_return_int(-1); + } +} diff -up ./src/Makefile.in.symbolic-link-attack-2 ./src/Makefile.in --- ./src/Makefile.in.symbolic-link-attack-2 2019-10-28 13:28:54.000000000 +0100 +++ ./src/Makefile.in 2021-02-02 15:31:20.555340446 +0100 @@ -120,16 +120,17 @@ SHELL = @SHELL@ PROGS = @PROGS@ -OBJS = conversation.o env_hooks.o exec.o exec_common.o exec_monitor.o \ - exec_nopty.o exec_pty.o get_pty.o hooks.o limits.o load_plugins.o \ - net_ifs.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@ +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 \ + limits.o load_plugins.o net_ifs.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@ IOBJS = $(OBJS:.o=.i) sesh.i POBJS = $(IOBJS:.i=.plog) -SESH_OBJS = sesh.o exec_common.o +SESH_OBJS = copy_file.o exec_common.o sesh.o CHECK_NOEXEC_OBJS = check_noexec.o exec_common.o @@ -335,6 +336,22 @@ conversation.i: $(srcdir)/conversation.c $(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_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_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 \ diff -up ./src/sesh.c.symbolic-link-attack-2 ./src/sesh.c --- ./src/sesh.c.symbolic-link-attack-2 2019-10-28 13:28:52.000000000 +0100 +++ ./src/sesh.c 2021-02-02 15:31:20.555340446 +0100 @@ -1,7 +1,7 @@ /* * SPDX-License-Identifier: ISC * - * Copyright (c) 2008, 2010-2018 Todd C. Miller + * Copyright (c) 2008, 2010-2018, 2020 Todd C. Miller * * Permission to use, copy, modify, and distribute this software for any * purpose with or without fee is hereby granted, provided that the above @@ -182,7 +182,7 @@ sesh_sudoedit(int argc, char *argv[]) * so that it's ensured that the temporary files are * created by us and that we are not opening any symlinks. */ - oflags_dst = O_WRONLY|O_TRUNC|O_CREAT|(post ? follow : O_EXCL); + oflags_dst = O_WRONLY|O_CREAT|(post ? follow : O_EXCL); for (i = 0; i < argc - 1; i += 2) { const char *path_src = argv[i]; const char *path_dst = argv[i + 1]; @@ -214,14 +214,29 @@ sesh_sudoedit(int argc, char *argv[]) } if (fd_src != -1) { - while ((nread = read(fd_src, buf, sizeof(buf))) > 0) { - if ((nwritten = write(fd_dst, buf, nread)) != nread) { - sudo_warn("%s", path_src); - if (post) { - ret = SESH_ERR_SOME_FILES; - goto nocleanup; - } else - goto cleanup_0; + off_t len_src = -1; + off_t len_dst = -1; + + if (post) { + if (fstat(fd_src, &sb) != 0) { + ret = SESH_ERR_SOME_FILES; + goto nocleanup; + } + len_src = sb.st_size; + if (fstat(fd_dst, &sb) != 0) { + ret = SESH_ERR_SOME_FILES; + goto nocleanup; + } + len_dst = sb.st_size; + } + + if (sudo_copy_file(path_src, fd_src, len_src, path_dst, fd_dst, + len_dst) == -1) { + if (post) { + ret = SESH_ERR_SOME_FILES; + goto nocleanup; + } else { + goto cleanup_0; } } } diff -up ./src/sudo_edit.c.symbolic-link-attack-2 ./src/sudo_edit.c --- ./src/sudo_edit.c.symbolic-link-attack-2 2021-02-02 15:31:20.554340459 +0100 +++ ./src/sudo_edit.c 2021-02-02 15:31:54.355884326 +0100 @@ -42,7 +42,6 @@ #include #include #include -#include #include #include "sudo.h" @@ -551,8 +550,6 @@ sudo_edit_create_tfiles(struct command_d struct tempfile *tf, char *files[], int nfiles) { int i, j, tfd, ofd, rc; - char buf[BUFSIZ]; - ssize_t nwritten, nread; struct timespec times[2]; struct stat sb; debug_decl(sudo_edit_create_tfiles, SUDO_DEBUG_EDIT) @@ -648,18 +645,7 @@ sudo_edit_create_tfiles(struct command_d debug_return_int(-1); } if (ofd != -1) { - while ((nread = read(ofd, buf, sizeof(buf))) > 0) { - if ((nwritten = write(tfd, buf, nread)) != nread) { - if (nwritten == -1) - sudo_warn("%s", tf[j].tfile); - else - sudo_warnx(U_("%s: short write"), tf[j].tfile); - break; - } - } - if (nread != 0) { - if (nread < 0) - sudo_warn("%s", files[i]); + if (sudo_copy_file(tf[j].ofile, ofd, tf[j].osize, tf[j].tfile, tfd, -1) == -1) { close(ofd); close(tfd); debug_return_int(-1); @@ -689,51 +675,6 @@ sudo_edit_create_tfiles(struct command_d } /* - * Extend the given fd to the specified size in bytes. - * We do this to allocate disk space up-front before overwriting - * the original file with the temporary. Otherwise, we could - * we run out of disk space after truncating the original file. - */ -static int -sudo_edit_extend_file(int fd, off_t new_size) -{ - off_t old_size, size; - ssize_t nwritten; - char zeroes[1024] = { '\0' }; - debug_decl(sudo_edit_extend_file, SUDO_DEBUG_EDIT); - - if ((old_size = lseek(fd, 0, SEEK_END)) == -1) { - sudo_warn("lseek"); - debug_return_int(-1); - } - sudo_debug_printf(SUDO_DEBUG_INFO, "%s: extending file from %lld to %lld", - __func__, (long long)old_size, (long long)new_size); - - for (size = old_size; size < new_size; size += nwritten) { - size_t len = new_size - size; - if (len > sizeof(zeroes)) - len = sizeof(zeroes); - nwritten = write(fd, zeroes, len); - if (nwritten == -1) { - int serrno = errno; - if (ftruncate(fd, old_size) == -1) { - sudo_debug_printf( - SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO|SUDO_DEBUG_ERRNO, - "unable to truncate to %lld", (long long)old_size); - } - errno = serrno; - debug_return_int(-1); - } - } - if (lseek(fd, 0, SEEK_SET) == -1) { - sudo_warn("lseek"); - debug_return_int(-1); - } - - debug_return_int(0); -} - -/* * Copy the temporary files specified in tf to the originals. * Returns the number of copy errors or 0 if completely successful. */ @@ -741,9 +682,7 @@ static int sudo_edit_copy_tfiles(struct command_details *command_details, struct tempfile *tf, int nfiles, struct timespec *times) { - int i, tfd, ofd, rc, errors = 0; - char buf[BUFSIZ]; - ssize_t nwritten, nread; + int i, tfd, ofd, errors = 0; struct timespec ts; struct stat sb; mode_t oldmask; @@ -751,7 +690,7 @@ sudo_edit_copy_tfiles(struct command_det /* Copy contents of temp files to real ones. */ for (i = 0; i < nfiles; i++) { - rc = -1; + int rc = -1; sudo_debug_printf(SUDO_DEBUG_INFO|SUDO_DEBUG_LINENO, "seteuid(%u)", (unsigned int)user_details.uid); if (seteuid(user_details.uid) != 0) @@ -764,8 +703,8 @@ sudo_edit_copy_tfiles(struct command_det "seteuid(%u)", ROOT_UID); if (seteuid(ROOT_UID) != 0) sudo_fatal("seteuid(ROOT_UID)"); - if (rc || !S_ISREG(sb.st_mode)) { - if (rc) + if (rc == -1 || !S_ISREG(sb.st_mode)) { + if (rc == -1) sudo_warn("%s", tf[i].tfile); else sudo_warnx(U_("%s: not a regular file"), tf[i].tfile); @@ -796,46 +735,19 @@ sudo_edit_copy_tfiles(struct command_det umask(oldmask); switch_user(ROOT_UID, user_details.egid, user_details.ngroups, user_details.groups); - if (ofd == -1) - goto write_error; - /* Extend the file to the new size if larger before copying. */ - if (tf[i].osize > 0 && sb.st_size > tf[i].osize) { - if (sudo_edit_extend_file(ofd, sb.st_size) == -1) - goto write_error; + if (ofd == -1) { + sudo_warn(U_("unable to write to %s"), tf[i].ofile); + goto bad; } + /* Overwrite the old file with the new contents. */ - while ((nread = read(tfd, buf, sizeof(buf))) > 0) { - ssize_t off = 0; - do { - nwritten = write(ofd, buf + off, nread - off); - if (nwritten == -1) - goto write_error; - off += nwritten; - } while (nread > off); - } - if (nread == 0) { - /* success, read to EOF */ - if (tf[i].osize > 0 && sb.st_size < tf[i].osize) { - /* We don't open with O_TRUNC so must truncate manually. */ - if (ftruncate(ofd, sb.st_size) == -1) { - sudo_debug_printf( - SUDO_DEBUG_ERROR|SUDO_DEBUG_LINENO|SUDO_DEBUG_ERRNO, - "unable to truncate %s to %lld", tf[i].ofile, - (long long)sb.st_size); - goto write_error; - } - } - unlink(tf[i].tfile); - } else if (nread < 0) { - sudo_warn(U_("unable to read temporary file")); - sudo_warnx(U_("contents of edit session left in %s"), tf[i].tfile); - errors++; - } else { -write_error: - sudo_warn(U_("unable to write to %s"), tf[i].ofile); + if (sudo_copy_file(tf[i].tfile, tfd, sb.st_size, tf[i].ofile, ofd, + tf[i].osize) == -1) { +bad: sudo_warnx(U_("contents of edit session left in %s"), tf[i].tfile); errors++; } + if (ofd != -1) close(ofd); close(tfd); diff -up ./src/sudo_exec.h.symbolic-link-attack-2 ./src/sudo_exec.h --- ./src/sudo_exec.h.symbolic-link-attack-2 2019-10-28 13:27:39.000000000 +0100 +++ ./src/sudo_exec.h 2021-02-02 15:31:20.556340432 +0100 @@ -84,6 +84,9 @@ struct command_details; struct command_status; +/* copy_file.c */ +int sudo_copy_file(const char *src, int src_fd, off_t src_len, const char *dst, int dst_fd, off_t dst_len); + /* exec.c */ void exec_cmnd(struct command_details *details, int errfd); void terminate_command(pid_t pid, bool use_pgrp);