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.
This commit is contained in:
Todd C. Miller
2023-07-26 19:43:49 -06:00
parent 14f1a12e2d
commit 760c9c1107
4 changed files with 58 additions and 28 deletions

View File

@@ -376,6 +376,28 @@ sudo_needs_pty(const struct command_details *details)
return false; 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 * 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, * sudoedit, there is no command timeout and there is no close function,

View File

@@ -467,12 +467,12 @@ write_callback(int fd, int what, void *v)
* ourselves using a pipe. Fills in io_pipe[][]. * ourselves using a pipe. Fills in io_pipe[][].
*/ */
static void 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 }; bool interpose[3] = { false, false, false };
struct stat sb, tty_sbuf, *tty_sb = NULL;
struct plugin_container *plugin; struct plugin_container *plugin;
bool want_winch = false; bool want_winch = false;
struct stat sb;
debug_decl(interpose_pipes, SUDO_DEBUG_EXEC); 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, * If stdin, stdout or stderr is not the user's tty and logging is
* use a pipe to interpose ourselves. * enabled, use a pipe to interpose ourselves.
*/ */
if (tty != NULL && stat(tty, &tty_sbuf) != -1)
tty_sb = &tty_sbuf;
if (interpose[STDIN_FILENO]) { 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, 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) if (pipe2(io_pipe[STDIN_FILENO], O_CLOEXEC) != 0)
sudo_fatal("%s", U_("unable to create pipe")); sudo_fatal("%s", U_("unable to create pipe"));
io_buf_new(STDIN_FILENO, io_pipe[STDIN_FILENO][1], 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 (interpose[STDOUT_FILENO]) {
if (!sudo_isatty(STDOUT_FILENO, &sb)) { if (!fd_matches_tty(STDOUT_FILENO, tty_sb, &sb)) {
sudo_debug_printf(SUDO_DEBUG_INFO, 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) if (pipe2(io_pipe[STDOUT_FILENO], O_CLOEXEC) != 0)
sudo_fatal("%s", U_("unable to create pipe")); sudo_fatal("%s", U_("unable to create pipe"));
io_buf_new(io_pipe[STDOUT_FILENO][0], STDOUT_FILENO, 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 (interpose[STDERR_FILENO]) {
if (!sudo_isatty(STDERR_FILENO, &sb)) { if (!fd_matches_tty(STDERR_FILENO, tty_sb, &sb)) {
sudo_debug_printf(SUDO_DEBUG_INFO, 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) if (pipe2(io_pipe[STDERR_FILENO], O_CLOEXEC) != 0)
sudo_fatal("%s", U_("unable to create pipe")); sudo_fatal("%s", U_("unable to create pipe"));
io_buf_new(io_pipe[STDERR_FILENO][0], STDERR_FILENO, 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 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 * Block signals until we have our handlers setup in the parent so

View File

@@ -1069,6 +1069,7 @@ exec_pty(struct command_details *details,
{ {
int io_pipe[3][2] = { { -1, -1 }, { -1, -1 }, { -1, -1 } }; int io_pipe[3][2] = { { -1, -1 }, { -1, -1 }, { -1, -1 } };
bool interpose[3] = { false, false, false }; bool interpose[3] = { false, false, false };
struct stat sb, tty_sbuf, *tty_sb = NULL;
int sv[2], intercept_sv[2] = { -1, -1 }; int sv[2], intercept_sv[2] = { -1, -1 };
struct exec_closure ec = { 0 }; struct exec_closure ec = { 0 };
struct plugin_container *plugin; struct plugin_container *plugin;
@@ -1076,7 +1077,6 @@ exec_pty(struct command_details *details,
bool cmnd_foreground; bool cmnd_foreground;
sigset_t set, oset; sigset_t set, oset;
struct sigaction sa; struct sigaction sa;
struct stat sb;
pid_t ppgrp, sudo_pid; pid_t ppgrp, sudo_pid;
debug_decl(exec_pty, SUDO_DEBUG_EXEC); 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, * If stdin, stdout or stderr is not the user's tty and logging is
* use a pipe to interpose ourselves instead of using the pty fd. * enabled, use a pipe to interpose ourselves instead of using the
* We always use a pipe for stdin when in background mode. * 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]) { if (!interpose[STDIN_FILENO]) {
/* Not logging stdin, do not interpose. */ /* Not logging stdin, do not interpose. */
sudo_debug_printf(SUDO_DEBUG_INFO, 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)) if (S_ISFIFO(sb.st_mode))
SET(details->flags, CD_EXEC_BG); SET(details->flags, CD_EXEC_BG);
io_fds[SFD_STDIN] = dup(STDIN_FILENO); io_fds[SFD_STDIN] = dup(STDIN_FILENO);
@@ -1190,7 +1193,7 @@ exec_pty(struct command_details *details,
sudo_fatal("dup"); sudo_fatal("dup");
} else { } else {
sudo_debug_printf(SUDO_DEBUG_INFO, 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); SET(details->flags, CD_EXEC_BG);
if (pipe2(io_pipe[STDIN_FILENO], O_CLOEXEC) != 0) if (pipe2(io_pipe[STDIN_FILENO], O_CLOEXEC) != 0)
sudo_fatal("%s", U_("unable to create pipe")); sudo_fatal("%s", U_("unable to create pipe"));
@@ -1225,11 +1228,11 @@ exec_pty(struct command_details *details,
close(io_pipe[STDIN_FILENO][1]); close(io_pipe[STDIN_FILENO][1]);
io_pipe[STDIN_FILENO][1] = -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]) { if (!interpose[STDOUT_FILENO]) {
/* Not logging stdout, do not interpose. */ /* Not logging stdout, do not interpose. */
sudo_debug_printf(SUDO_DEBUG_INFO, 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)) { if (S_ISFIFO(sb.st_mode)) {
SET(details->flags, CD_EXEC_BG); SET(details->flags, CD_EXEC_BG);
term_raw_flags = SUDO_TERM_OFLAG; term_raw_flags = SUDO_TERM_OFLAG;
@@ -1239,7 +1242,7 @@ exec_pty(struct command_details *details,
sudo_fatal("dup"); sudo_fatal("dup");
} else { } else {
sudo_debug_printf(SUDO_DEBUG_INFO, 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); SET(details->flags, CD_EXEC_BG);
term_raw_flags = SUDO_TERM_OFLAG; term_raw_flags = SUDO_TERM_OFLAG;
if (pipe2(io_pipe[STDOUT_FILENO], O_CLOEXEC) != 0) 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]; 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]) { if (!interpose[STDERR_FILENO]) {
/* Not logging stderr, do not interpose. */ /* Not logging stderr, do not interpose. */
sudo_debug_printf(SUDO_DEBUG_INFO, 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); io_fds[SFD_STDERR] = dup(STDERR_FILENO);
if (io_fds[SFD_STDERR] == -1) if (io_fds[SFD_STDERR] == -1)
sudo_fatal("dup"); sudo_fatal("dup");
} else { } else {
sudo_debug_printf(SUDO_DEBUG_INFO, 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) if (pipe2(io_pipe[STDERR_FILENO], O_CLOEXEC) != 0)
sudo_fatal("%s", U_("unable to create pipe")); sudo_fatal("%s", U_("unable to create pipe"));
io_buf_new(io_pipe[STDERR_FILENO][0], STDERR_FILENO, 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); sudo_fatal_callback_deregister(pty_cleanup_hook);
/* /*
* If stdin/stdout is not a tty, start command in the background * If stdin/stdout is not the user's tty, start the command in
* since it might be part of a pipeline that reads from /dev/tty. * the background since it might be part of a pipeline that reads
* In this case, we rely on the command receiving SIGTTOU or SIGTTIN * from /dev/tty. In this case, we rely on the command receiving
* when it needs access to the controlling tty. * SIGTTOU or SIGTTIN when it needs access to the controlling tty.
*/ */
exec_monitor(details, &oset, cmnd_foreground, sv[1], intercept_sv[1]); exec_monitor(details, &oset, cmnd_foreground, sv[1], intercept_sv[1]);
cstat->type = CMD_ERRNO; cstat->type = CMD_ERRNO;

View File

@@ -174,10 +174,12 @@ union sudo_token_un {
#endif /* _PATH_SUDO_INTERCEPT && __linux__ */ #endif /* _PATH_SUDO_INTERCEPT && __linux__ */
/* exec.c */ /* exec.c */
struct stat;
void exec_cmnd(struct command_details *details, sigset_t *mask, int intercept_fd, int errfd); void exec_cmnd(struct command_details *details, sigset_t *mask, int intercept_fd, int errfd);
void terminate_command(pid_t pid, bool use_pgrp); void terminate_command(pid_t pid, bool use_pgrp);
bool sudo_terminated(struct command_status *cstat); bool sudo_terminated(struct command_status *cstat);
void free_exec_closure(struct exec_closure *ec); 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 */ /* exec_common.c */
int sudo_execve(int fd, const char *path, char *const argv[], char *envp[], int intercept_fd, unsigned int flags); int sudo_execve(int fd, const char *path, char *const argv[], char *envp[], int intercept_fd, unsigned int flags);