Skip to content

Commit fd3ff3a

Browse files
committed
Check the controlling tty to determine if a tty belongs to the user.
Previously, we compared the terminal device number returned by get_process_ttyname() with that of stdin, stdout and stderr. This causes problems on Linux if the user is logged in on the console, which is a virtual device that may correspond to one of several different terminal devices. In this specific case, there is a mismatch between the controlling terminal listed in /proc/self/stat (which corresponds to the underlying terminal device) and the device number of stdin, stdout and stderr (which is that of /dev/console).
1 parent a3cd820 commit fd3ff3a

File tree

4 files changed

+20
-28
lines changed

4 files changed

+20
-28
lines changed

src/exec.c

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -377,26 +377,23 @@ sudo_needs_pty(const struct command_details *details)
377377
}
378378

379379
/*
380-
* Check whether the specified fd matches the device file that
381-
* corresponds to tty_sb. If tty_sb is NULL, just check whether
382-
* fd is a tty. Always fills in fd_sb (zeroed on error).
380+
* Check whether the specified fd is a terminal with the specified
381+
* controlling process group. Always fills in sb (zeroed on error).
383382
* Returns true on match, else false.
384383
*/
385384
bool
386-
fd_matches_tty(int fd, struct stat *tty_sb, struct stat *fd_sb)
385+
fd_matches_pgrp(int fd, pid_t pgrp, struct stat *sb)
387386
{
388-
debug_decl(fd_is_user_tty, SUDO_DEBUG_EXEC);
387+
debug_decl(fd_matches_pgrp, SUDO_DEBUG_EXEC);
389388

390-
if (fstat(fd, fd_sb) == -1) {
391-
/* Always initialize fd_sb. */
392-
memset(fd_sb, 0, sizeof(*fd_sb));
393-
debug_return_bool(false);
394-
}
395-
if (!S_ISCHR(fd_sb->st_mode))
389+
if (!sudo_isatty(fd, sb))
396390
debug_return_bool(false);
397391

398-
/* Compare with tty_sb if available, else just check that fd is a tty. */
399-
debug_return_bool(tty_sb ? tty_sb->st_rdev == fd_sb->st_rdev : isatty(fd));
392+
if (pgrp != -1) {
393+
if (pgrp != tcgetpgrp(fd))
394+
debug_return_bool(false);
395+
}
396+
debug_return_bool(true);
400397
}
401398

402399
/*

src/exec_nopty.c

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -470,9 +470,10 @@ static void
470470
interpose_pipes(struct exec_closure *ec, const char *tty, int io_pipe[3][2])
471471
{
472472
bool interpose[3] = { false, false, false };
473-
struct stat sb, tty_sbuf, *tty_sb = NULL;
474473
struct plugin_container *plugin;
474+
const pid_t pgrp = getpgrp();
475475
bool want_winch = false;
476+
struct stat sb;
476477
debug_decl(interpose_pipes, SUDO_DEBUG_EXEC);
477478

478479
/*
@@ -496,11 +497,8 @@ interpose_pipes(struct exec_closure *ec, const char *tty, int io_pipe[3][2])
496497
* If stdin, stdout or stderr is not the user's tty and logging is
497498
* enabled, use a pipe to interpose ourselves.
498499
*/
499-
if (tty != NULL && stat(tty, &tty_sbuf) != -1)
500-
tty_sb = &tty_sbuf;
501-
502500
if (interpose[STDIN_FILENO]) {
503-
if (!fd_matches_tty(STDIN_FILENO, tty_sb, &sb)) {
501+
if (!fd_matches_pgrp(STDIN_FILENO, pgrp, &sb)) {
504502
sudo_debug_printf(SUDO_DEBUG_INFO,
505503
"stdin not user's tty, creating a pipe");
506504
if (pipe2(io_pipe[STDIN_FILENO], O_CLOEXEC) != 0)
@@ -510,7 +508,7 @@ interpose_pipes(struct exec_closure *ec, const char *tty, int io_pipe[3][2])
510508
}
511509
}
512510
if (interpose[STDOUT_FILENO]) {
513-
if (!fd_matches_tty(STDOUT_FILENO, tty_sb, &sb)) {
511+
if (!fd_matches_pgrp(STDOUT_FILENO, pgrp, &sb)) {
514512
sudo_debug_printf(SUDO_DEBUG_INFO,
515513
"stdout not user's tty, creating a pipe");
516514
if (pipe2(io_pipe[STDOUT_FILENO], O_CLOEXEC) != 0)
@@ -520,7 +518,7 @@ interpose_pipes(struct exec_closure *ec, const char *tty, int io_pipe[3][2])
520518
}
521519
}
522520
if (interpose[STDERR_FILENO]) {
523-
if (!fd_matches_tty(STDERR_FILENO, tty_sb, &sb)) {
521+
if (!fd_matches_pgrp(STDERR_FILENO, pgrp, &sb)) {
524522
sudo_debug_printf(SUDO_DEBUG_INFO,
525523
"stderr not user's tty, creating a pipe");
526524
if (pipe2(io_pipe[STDERR_FILENO], O_CLOEXEC) != 0)

src/exec_pty.c

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1095,7 +1095,7 @@ exec_pty(struct command_details *details,
10951095
{
10961096
int io_pipe[3][2] = { { -1, -1 }, { -1, -1 }, { -1, -1 } };
10971097
bool interpose[3] = { false, false, false };
1098-
struct stat sb, tty_sbuf, *tty_sb = NULL;
1098+
struct stat sb;
10991099
int sv[2], intercept_sv[2] = { -1, -1 };
11001100
struct exec_closure *ec = &pty_ec;
11011101
struct plugin_container *plugin;
@@ -1203,10 +1203,7 @@ exec_pty(struct command_details *details,
12031203
* enabled, use a pipe to interpose ourselves instead of using the
12041204
* pty fd. We always use a pipe for stdin when in background mode.
12051205
*/
1206-
if (user_details->tty != NULL && stat(user_details->tty, &tty_sbuf) != -1)
1207-
tty_sb = &tty_sbuf;
1208-
1209-
if (!fd_matches_tty(STDIN_FILENO, tty_sb, &sb)) {
1206+
if (!fd_matches_pgrp(STDIN_FILENO, ppgrp, &sb)) {
12101207
if (!interpose[STDIN_FILENO]) {
12111208
/* Not logging stdin, do not interpose. */
12121209
sudo_debug_printf(SUDO_DEBUG_INFO,
@@ -1253,7 +1250,7 @@ exec_pty(struct command_details *details,
12531250
close(io_pipe[STDIN_FILENO][1]);
12541251
io_pipe[STDIN_FILENO][1] = -1;
12551252
}
1256-
if (!fd_matches_tty(STDOUT_FILENO, tty_sb, &sb)) {
1253+
if (!fd_matches_pgrp(STDOUT_FILENO, ppgrp, &sb)) {
12571254
if (!interpose[STDOUT_FILENO]) {
12581255
/* Not logging stdout, do not interpose. */
12591256
sudo_debug_printf(SUDO_DEBUG_INFO,
@@ -1277,7 +1274,7 @@ exec_pty(struct command_details *details,
12771274
io_fds[SFD_STDOUT] = io_pipe[STDOUT_FILENO][1];
12781275
}
12791276
}
1280-
if (!fd_matches_tty(STDERR_FILENO, tty_sb, &sb)) {
1277+
if (!fd_matches_pgrp(STDERR_FILENO, ppgrp, &sb)) {
12811278
if (!interpose[STDERR_FILENO]) {
12821279
/* Not logging stderr, do not interpose. */
12831280
sudo_debug_printf(SUDO_DEBUG_INFO,

src/sudo_exec.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ void exec_cmnd(struct command_details *details, sigset_t *mask, int intercept_fd
179179
void terminate_command(pid_t pid, bool use_pgrp);
180180
bool sudo_terminated(struct command_status *cstat);
181181
void free_exec_closure(struct exec_closure *ec);
182-
bool fd_matches_tty(int fd, struct stat *tty_sb, struct stat *fd_sb);
182+
bool fd_matches_pgrp(int fd, pid_t pgrp, struct stat *sb);
183183

184184
/* exec_common.c */
185185
int sudo_execve(int fd, const char *path, char *const argv[], char *envp[], int intercept_fd, unsigned int flags);

0 commit comments

Comments
 (0)