Avoid calling isatty()/ttyname() on std{in,out,err} if not a char dev.

The user controls these fds so we should avoid calling ioctl(2) on
them unless they correspond to actual character device files.

5 files changed, 59 insertions, 26 deletions

diff --git a/src/exec_nopty.c b/src/exec_nopty.c
index 3c78e4415..db9a21487 100644
--- a/src/exec_nopty.c
+++ b/src/exec_nopty.c
@@ -472,6 +472,7 @@ interpose_pipes(struct exec_closure *ec, int io_pipe[3][2])
     bool interpose[3] = { false, false, false };
     struct plugin_container *plugin;
     bool want_winch = false;
+    struct stat sb;
     debug_decl(interpose_pipes, SUDO_DEBUG_EXEC);
 
     /*
@@ -496,7 +497,7 @@ interpose_pipes(struct exec_closure *ec, int io_pipe[3][2])
      * use a pipe to interpose ourselves.
      */
     if (interpose[STDIN_FILENO]) {
-	if (!isatty(STDIN_FILENO)) {
+	if (!sudo_isatty(STDIN_FILENO, &sb)) {
 	    sudo_debug_printf(SUDO_DEBUG_INFO,
 		"stdin not a tty, creating a pipe");
 	    if (pipe2(io_pipe[STDIN_FILENO], O_CLOEXEC) != 0)
@@ -506,7 +507,7 @@ interpose_pipes(struct exec_closure *ec, int io_pipe[3][2])
 	}
     }
     if (interpose[STDOUT_FILENO]) {
-	if (!isatty(STDOUT_FILENO)) {
+	if (!sudo_isatty(STDOUT_FILENO, &sb)) {
 	    sudo_debug_printf(SUDO_DEBUG_INFO,
 		"stdout not a tty, creating a pipe");
 	    if (pipe2(io_pipe[STDOUT_FILENO], O_CLOEXEC) != 0)
@@ -516,7 +517,7 @@ interpose_pipes(struct exec_closure *ec, int io_pipe[3][2])
 	}
     }
     if (interpose[STDERR_FILENO]) {
-	if (!isatty(STDERR_FILENO)) {
+	if (!sudo_isatty(STDERR_FILENO, &sb)) {
 	    sudo_debug_printf(SUDO_DEBUG_INFO,
 		"stderr not a tty, creating a pipe");
 	    if (pipe2(io_pipe[STDERR_FILENO], O_CLOEXEC) != 0)
diff --git a/src/exec_pty.c b/src/exec_pty.c
index dc58138f6..676609063 100644
--- a/src/exec_pty.c
+++ b/src/exec_pty.c
@@ -1175,13 +1175,14 @@ 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 (!isatty(STDIN_FILENO)) {
+    if (!sudo_isatty(STDIN_FILENO, &sb)) {
 	if (!interpose[STDIN_FILENO]) {
 	    /* Not logging stdin, do not interpose. */
 	    sudo_debug_printf(SUDO_DEBUG_INFO,
 		"stdin not a tty, not logging");
-	    if (fstat(STDIN_FILENO, &sb) == 0 && S_ISFIFO(sb.st_mode))
+	    if (S_ISFIFO(sb.st_mode))
 		pipeline = true;
 	    io_fds[SFD_STDIN] = dup(STDIN_FILENO);
 	    if (io_fds[SFD_STDIN] == -1)
@@ -1223,12 +1224,12 @@ exec_pty(struct command_details *details,
 	    close(io_pipe[STDIN_FILENO][1]);
 	    io_pipe[STDIN_FILENO][1] = -1;
     }
-    if (!isatty(STDOUT_FILENO)) {
+    if (!sudo_isatty(STDOUT_FILENO, &sb)) {
 	if (!interpose[STDOUT_FILENO]) {
 	    /* Not logging stdout, do not interpose. */
 	    sudo_debug_printf(SUDO_DEBUG_INFO,
 		"stdout not a tty, not logging");
-	    if (fstat(STDOUT_FILENO, &sb) == 0 && S_ISFIFO(sb.st_mode))
+	    if (S_ISFIFO(sb.st_mode))
 		pipeline = true;
 	    io_fds[SFD_STDOUT] = dup(STDOUT_FILENO);
 	    if (io_fds[SFD_STDOUT] == -1)
@@ -1244,12 +1245,12 @@ exec_pty(struct command_details *details,
 	    io_fds[SFD_STDOUT] = io_pipe[STDOUT_FILENO][1];
 	}
     }
-    if (!isatty(STDERR_FILENO)) {
+    if (!sudo_isatty(STDERR_FILENO, &sb)) {
 	if (!interpose[STDERR_FILENO]) {
 	    /* Not logging stderr, do not interpose. */
 	    sudo_debug_printf(SUDO_DEBUG_INFO,
 		"stderr not a tty, not logging");
-	    if (fstat(STDERR_FILENO, &sb) == 0 && S_ISFIFO(sb.st_mode))
+	    if (S_ISFIFO(sb.st_mode))
 		pipeline = true;
 	    io_fds[SFD_STDERR] = dup(STDERR_FILENO);
 	    if (io_fds[SFD_STDERR] == -1)
diff --git a/src/selinux.c b/src/selinux.c
index 50455f4d0..153bd58d4 100644
--- a/src/selinux.c
+++ b/src/selinux.c
@@ -273,7 +273,7 @@ selinux_relabel_tty(const char *ttyn, int ptyfd)
 	(void)fcntl(se_state.ttyfd, F_SETFL,
 	    fcntl(se_state.ttyfd, F_GETFL, 0) & ~O_NONBLOCK);
 	for (fd = STDIN_FILENO; fd <= STDERR_FILENO; fd++) {
-	    if (isatty(fd) && dup2(se_state.ttyfd, fd) == -1) {
+	    if (sudo_isatty(fd, &sb) && dup2(se_state.ttyfd, fd) == -1) {
 		sudo_warn("dup2");
 		goto bad;
 	    }
diff --git a/src/sudo.h b/src/sudo.h
index 4bec1bb54..b5342c4f0 100644
--- a/src/sudo.h
+++ b/src/sudo.h
@@ -318,6 +318,7 @@ int get_net_ifs(char **addrinfo);
 
 /* ttyname.c */
 char *get_process_ttyname(char *name, size_t namelen);
+bool sudo_isatty(int fd, struct stat *sb);
 
 /* signal.c */
 struct sigaction;
diff --git a/src/ttyname.c b/src/ttyname.c
index 12432a507..2a5c288fc 100644
--- a/src/ttyname.c
+++ b/src/ttyname.c
@@ -185,7 +185,7 @@ get_process_ttyname(char *name, size_t namelen)
 
 	/* Missing /proc/pid/psinfo file. */
 	for (i = STDIN_FILENO; i <= STDERR_FILENO; i++) {
-	    if (isatty(i) && fstat(i, &sb) != -1) {
+	    if (sudo_isatty(i, &sb)) {
 		ret = sudo_ttyname_dev(sb.st_rdev, name, namelen);
 		goto done;
 	    }
@@ -286,7 +286,7 @@ get_process_ttyname(char *name, size_t namelen)
 
 	/* No parent pid found, /proc/self/stat is missing or corrupt. */
 	for (i = STDIN_FILENO; i <= STDERR_FILENO; i++) {
-	    if (isatty(i) && fstat(i, &sb) != -1) {
+	    if (sudo_isatty(i, &sb)) {
 		ret = sudo_ttyname_dev(sb.st_rdev, name, namelen);
 		goto done;
 	    }
@@ -347,25 +347,55 @@ done:
 char *
 get_process_ttyname(char *name, size_t namelen)
 {
+    struct stat sb;
     char *tty;
+    int i;
     debug_decl(get_process_ttyname, SUDO_DEBUG_UTIL);
 
-    if ((tty = ttyname(STDIN_FILENO)) == NULL) {
-	if ((tty = ttyname(STDOUT_FILENO)) == NULL)
-	    tty = ttyname(STDERR_FILENO);
-    }
-    if (tty != NULL) {
-	if (strlcpy(name, tty, namelen) < namelen)
-	    debug_return_str(name);
-	errno = ERANGE;
-	sudo_debug_printf(SUDO_DEBUG_WARN|SUDO_DEBUG_LINENO|SUDO_DEBUG_ERRNO,
-	    "unable to store tty from ttyname");
-    } else {
-	sudo_debug_printf(SUDO_DEBUG_WARN|SUDO_DEBUG_LINENO|SUDO_DEBUG_ERRNO,
-	    "unable to resolve tty via ttyname");
-	errno = ENOENT;
+    for (i = STDIN_FILENO; i <= STDERR_FILENO; i++) {
+	/* Only call ttyname() on a character special device. */
+	if (fstat(i, &sb) == -1 || !S_ISCHR(sb.st_mode))
+	    continue;
+	if ((tty = ttyname(i)) == NULL)
+	    continue;
+
+	if (strlcpy(name, tty, namelen) >= namelen) {
+	    errno = ENAMETOOLONG;
+	    sudo_debug_printf(
+		SUDO_DEBUG_WARN|SUDO_DEBUG_LINENO|SUDO_DEBUG_ERRNO,
+		"unable to store tty from ttyname");
+	    debug_return_str(NULL);
+	}
+	debug_return_str(name);
     }
 
+    sudo_debug_printf(SUDO_DEBUG_WARN|SUDO_DEBUG_LINENO|SUDO_DEBUG_ERRNO,
+	"unable to resolve tty via ttyname");
+    errno = ENOENT;
     debug_return_str(NULL);
 }
 #endif
+
+/*
+ * Like isatty(3) but stats the fd and stores the result in sb.
+ * Only calls isatty(3) if fd is a character special device.
+ * Returns true if a tty, else returns false and sets errno.
+ */
+bool
+sudo_isatty(int fd, struct stat *sb)
+{
+    bool ret = false;
+    debug_decl(sudo_isatty, SUDO_DEBUG_EXEC);
+
+    if (fstat(fd, sb) == 0) {
+        if (!S_ISCHR(sb->st_mode)) {
+            errno = ENOTTY;
+        } else {
+            ret = isatty(fd) == 1;
+        }
+    } else {
+	/* Always initialize sb. */
+	memset(sb, 0, sizeof(*sb));
+    }
+    debug_return_bool(ret);
+}