From 760c9c11074cb921ecc0da9fbb5f0a12afd46233 Mon Sep 17 00:00:00 2001 From: "Todd C. Miller" Date: Wed, 26 Jul 2023 19:43:49 -0600 Subject: [PATCH] Don't assume that if std{in,out,err} is a tty, it is the user's tty. Previously, sudo only checked that the fd was a terminal, not that it matched sudo's idea of the user's terminal. This matters when input or output is redirected to a different terminal. In that case we want to interpose the fd with a pipe even if it refers to a terminal. Bug #1056. --- src/exec.c | 22 ++++++++++++++++++++++ src/exec_nopty.c | 25 ++++++++++++++----------- src/exec_pty.c | 37 ++++++++++++++++++++----------------- src/sudo_exec.h | 2 ++ 4 files changed, 58 insertions(+), 28 deletions(-) diff --git a/src/exec.c b/src/exec.c index 1098fd8c1..b53587de0 100644 --- a/src/exec.c +++ b/src/exec.c @@ -376,6 +376,28 @@ sudo_needs_pty(const struct command_details *details) return false; } +/* + * Check whether the specified fd matches the device file that + * corresponds to tty_sb. If tty_sb is NULL, just check whether + * fd is a tty. Always fills in fd_sb (zeroed on error). + * Returns true on match, else false. + */ +bool +fd_matches_tty(int fd, struct stat *tty_sb, struct stat *fd_sb) +{ + bool ret; + debug_decl(fd_is_user_tty, SUDO_DEBUG_EXEC); + + if (fstat(fd, fd_sb) == -1 || !S_ISCHR(fd_sb->st_mode)) { + /* Always initialize fd_sb. */ + memset(fd_sb, 0, sizeof(*fd_sb)); + debug_return_bool(false); + } + + /* Compare with tty_sb if available, else just check that fd is a tty. */ + debug_return_bool(tty_sb ? tty_sb->st_rdev == fd_sb->st_rdev : isatty(fd)); +} + /* * If we are not running the command in a pty, we were not invoked as * sudoedit, there is no command timeout and there is no close function, diff --git a/src/exec_nopty.c b/src/exec_nopty.c index 6cb695132..ffb9ac9c5 100644 --- a/src/exec_nopty.c +++ b/src/exec_nopty.c @@ -467,12 +467,12 @@ write_callback(int fd, int what, void *v) * ourselves using a pipe. Fills in io_pipe[][]. */ static void -interpose_pipes(struct exec_closure *ec, int io_pipe[3][2]) +interpose_pipes(struct exec_closure *ec, const char *tty, int io_pipe[3][2]) { bool interpose[3] = { false, false, false }; + struct stat sb, tty_sbuf, *tty_sb = NULL; struct plugin_container *plugin; bool want_winch = false; - struct stat sb; debug_decl(interpose_pipes, SUDO_DEBUG_EXEC); /* @@ -493,13 +493,16 @@ interpose_pipes(struct exec_closure *ec, int io_pipe[3][2]) } /* - * If stdin, stdout or stderr is not a tty and logging is enabled, - * use a pipe to interpose ourselves. + * If stdin, stdout or stderr is not the user's tty and logging is + * enabled, use a pipe to interpose ourselves. */ + if (tty != NULL && stat(tty, &tty_sbuf) != -1) + tty_sb = &tty_sbuf; + if (interpose[STDIN_FILENO]) { - if (!sudo_isatty(STDIN_FILENO, &sb)) { + if (!fd_matches_tty(STDIN_FILENO, tty_sb, &sb)) { sudo_debug_printf(SUDO_DEBUG_INFO, - "stdin not a tty, creating a pipe"); + "stdin not user's tty, creating a pipe"); if (pipe2(io_pipe[STDIN_FILENO], O_CLOEXEC) != 0) sudo_fatal("%s", U_("unable to create pipe")); io_buf_new(STDIN_FILENO, io_pipe[STDIN_FILENO][1], @@ -507,9 +510,9 @@ interpose_pipes(struct exec_closure *ec, int io_pipe[3][2]) } } if (interpose[STDOUT_FILENO]) { - if (!sudo_isatty(STDOUT_FILENO, &sb)) { + if (!fd_matches_tty(STDOUT_FILENO, tty_sb, &sb)) { sudo_debug_printf(SUDO_DEBUG_INFO, - "stdout not a tty, creating a pipe"); + "stdout not user's tty, creating a pipe"); if (pipe2(io_pipe[STDOUT_FILENO], O_CLOEXEC) != 0) sudo_fatal("%s", U_("unable to create pipe")); io_buf_new(io_pipe[STDOUT_FILENO][0], STDOUT_FILENO, @@ -517,9 +520,9 @@ interpose_pipes(struct exec_closure *ec, int io_pipe[3][2]) } } if (interpose[STDERR_FILENO]) { - if (!sudo_isatty(STDERR_FILENO, &sb)) { + if (!fd_matches_tty(STDERR_FILENO, tty_sb, &sb)) { sudo_debug_printf(SUDO_DEBUG_INFO, - "stderr not a tty, creating a pipe"); + "stderr not user's tty, creating a pipe"); if (pipe2(io_pipe[STDERR_FILENO], O_CLOEXEC) != 0) sudo_fatal("%s", U_("unable to create pipe")); io_buf_new(io_pipe[STDERR_FILENO][0], STDERR_FILENO, @@ -571,7 +574,7 @@ exec_nopty(struct command_details *details, } /* Interpose std{in,out,err} with pipes if logging I/O. */ - interpose_pipes(&ec, io_pipe); + interpose_pipes(&ec, user_details->tty, io_pipe); /* * Block signals until we have our handlers setup in the parent so diff --git a/src/exec_pty.c b/src/exec_pty.c index e8c71f3e0..2e65dda12 100644 --- a/src/exec_pty.c +++ b/src/exec_pty.c @@ -1069,6 +1069,7 @@ exec_pty(struct command_details *details, { int io_pipe[3][2] = { { -1, -1 }, { -1, -1 }, { -1, -1 } }; bool interpose[3] = { false, false, false }; + struct stat sb, tty_sbuf, *tty_sb = NULL; int sv[2], intercept_sv[2] = { -1, -1 }; struct exec_closure ec = { 0 }; struct plugin_container *plugin; @@ -1076,7 +1077,6 @@ exec_pty(struct command_details *details, bool cmnd_foreground; sigset_t set, oset; struct sigaction sa; - struct stat sb; pid_t ppgrp, sudo_pid; debug_decl(exec_pty, SUDO_DEBUG_EXEC); @@ -1174,15 +1174,18 @@ exec_pty(struct command_details *details, } /* - * If stdin, stdout or stderr is not a tty and logging is enabled, - * use a pipe to interpose ourselves instead of using the pty fd. - * We always use a pipe for stdin when in background mode. + * If stdin, stdout or stderr is not the user's tty and logging is + * enabled, use a pipe to interpose ourselves instead of using the + * pty fd. We always use a pipe for stdin when in background mode. */ - if (!sudo_isatty(STDIN_FILENO, &sb)) { + if (user_details->tty != NULL && stat(user_details->tty, &tty_sbuf) != -1) + tty_sb = &tty_sbuf; + + if (!fd_matches_tty(STDIN_FILENO, tty_sb, &sb)) { if (!interpose[STDIN_FILENO]) { /* Not logging stdin, do not interpose. */ sudo_debug_printf(SUDO_DEBUG_INFO, - "stdin not a tty, not logging"); + "stdin not user's tty, not logging"); if (S_ISFIFO(sb.st_mode)) SET(details->flags, CD_EXEC_BG); io_fds[SFD_STDIN] = dup(STDIN_FILENO); @@ -1190,7 +1193,7 @@ exec_pty(struct command_details *details, sudo_fatal("dup"); } else { sudo_debug_printf(SUDO_DEBUG_INFO, - "stdin not a tty, creating a pipe"); + "stdin not user's tty, creating a pipe"); SET(details->flags, CD_EXEC_BG); if (pipe2(io_pipe[STDIN_FILENO], O_CLOEXEC) != 0) sudo_fatal("%s", U_("unable to create pipe")); @@ -1225,11 +1228,11 @@ exec_pty(struct command_details *details, close(io_pipe[STDIN_FILENO][1]); io_pipe[STDIN_FILENO][1] = -1; } - if (!sudo_isatty(STDOUT_FILENO, &sb)) { + if (!fd_matches_tty(STDOUT_FILENO, tty_sb, &sb)) { if (!interpose[STDOUT_FILENO]) { /* Not logging stdout, do not interpose. */ sudo_debug_printf(SUDO_DEBUG_INFO, - "stdout not a tty, not logging"); + "stdout not user's tty, not logging"); if (S_ISFIFO(sb.st_mode)) { SET(details->flags, CD_EXEC_BG); term_raw_flags = SUDO_TERM_OFLAG; @@ -1239,7 +1242,7 @@ exec_pty(struct command_details *details, sudo_fatal("dup"); } else { sudo_debug_printf(SUDO_DEBUG_INFO, - "stdout not a tty, creating a pipe"); + "stdout not user's tty, creating a pipe"); SET(details->flags, CD_EXEC_BG); term_raw_flags = SUDO_TERM_OFLAG; if (pipe2(io_pipe[STDOUT_FILENO], O_CLOEXEC) != 0) @@ -1249,17 +1252,17 @@ exec_pty(struct command_details *details, io_fds[SFD_STDOUT] = io_pipe[STDOUT_FILENO][1]; } } - if (!sudo_isatty(STDERR_FILENO, &sb)) { + if (!fd_matches_tty(STDERR_FILENO, tty_sb, &sb)) { if (!interpose[STDERR_FILENO]) { /* Not logging stderr, do not interpose. */ sudo_debug_printf(SUDO_DEBUG_INFO, - "stderr not a tty, not logging"); + "stderr not user's tty, not logging"); io_fds[SFD_STDERR] = dup(STDERR_FILENO); if (io_fds[SFD_STDERR] == -1) sudo_fatal("dup"); } else { sudo_debug_printf(SUDO_DEBUG_INFO, - "stderr not a tty, creating a pipe"); + "stderr /dev/tty, creating a pipe"); if (pipe2(io_pipe[STDERR_FILENO], O_CLOEXEC) != 0) sudo_fatal("%s", U_("unable to create pipe")); io_buf_new(io_pipe[STDERR_FILENO][0], STDERR_FILENO, @@ -1319,10 +1322,10 @@ exec_pty(struct command_details *details, sudo_fatal_callback_deregister(pty_cleanup_hook); /* - * If stdin/stdout is not a tty, start command in the background - * since it might be part of a pipeline that reads from /dev/tty. - * In this case, we rely on the command receiving SIGTTOU or SIGTTIN - * when it needs access to the controlling tty. + * If stdin/stdout is not the user's tty, start the command in + * the background since it might be part of a pipeline that reads + * from /dev/tty. In this case, we rely on the command receiving + * SIGTTOU or SIGTTIN when it needs access to the controlling tty. */ exec_monitor(details, &oset, cmnd_foreground, sv[1], intercept_sv[1]); cstat->type = CMD_ERRNO; diff --git a/src/sudo_exec.h b/src/sudo_exec.h index 61b852bfb..7dbda8aa8 100644 --- a/src/sudo_exec.h +++ b/src/sudo_exec.h @@ -174,10 +174,12 @@ union sudo_token_un { #endif /* _PATH_SUDO_INTERCEPT && __linux__ */ /* exec.c */ +struct stat; void exec_cmnd(struct command_details *details, sigset_t *mask, int intercept_fd, int errfd); void terminate_command(pid_t pid, bool use_pgrp); bool sudo_terminated(struct command_status *cstat); void free_exec_closure(struct exec_closure *ec); +bool fd_matches_tty(int fd, struct stat *tty_sb, struct stat *fd_sb); /* exec_common.c */ int sudo_execve(int fd, const char *path, char *const argv[], char *envp[], int intercept_fd, unsigned int flags);