Use dup3() instead of dup2().

This is less error prone since dup3() returns an error if old == new.
Sudo guarantees that fds 0-2 are already open.
This commit is contained in:
Todd C. Miller
2019-11-02 10:55:50 -06:00
parent 43df086186
commit 356287557f
4 changed files with 35 additions and 46 deletions

View File

@@ -722,12 +722,8 @@ exec_mailer(int pipein)
#endif /* NO_ROOT_MAILER */ #endif /* NO_ROOT_MAILER */
debug_decl(exec_mailer, SUDOERS_DEBUG_LOGGING) debug_decl(exec_mailer, SUDOERS_DEBUG_LOGGING)
/* Set stdin to read side of the pipe or clear FD_CLOEXEC */ /* Set stdin to read side of the pipe. */
if (pipein == STDIN_FILENO) if (dup3(pipein, STDIN_FILENO, 0) == -1) {
i = fcntl(pipein, F_SETFD, 0);
else
i = dup2(pipein, STDIN_FILENO);
if (i == -1) {
mysyslog(LOG_ERR, _("unable to dup stdin: %m")); mysyslog(LOG_ERR, _("unable to dup stdin: %m"));
sudo_debug_printf(SUDO_DEBUG_ERROR, sudo_debug_printf(SUDO_DEBUG_ERROR,
"unable to dup stdin: %s", strerror(errno)); "unable to dup stdin: %s", strerror(errno));

View File

@@ -404,33 +404,18 @@ exec_cmnd_pty(struct command_details *details, bool foreground, int errfd)
setpgid(0, self); setpgid(0, self);
/* Wire up standard fds, note that stdout/stderr may be pipes. */ /* Wire up standard fds, note that stdout/stderr may be pipes. */
if (io_fds[SFD_STDIN] != STDIN_FILENO) { if (dup3(io_fds[SFD_STDIN], STDIN_FILENO, 0) == -1)
if (dup2(io_fds[SFD_STDIN], STDIN_FILENO) == -1) sudo_fatal("dup3");
sudo_fatal("dup2");
if (io_fds[SFD_STDIN] != io_fds[SFD_SLAVE]) if (io_fds[SFD_STDIN] != io_fds[SFD_SLAVE])
close(io_fds[SFD_STDIN]); close(io_fds[SFD_STDIN]);
} else { if (dup3(io_fds[SFD_STDOUT], STDOUT_FILENO, 0) == -1)
if (fcntl(io_fds[SFD_STDIN], F_SETFD, 0) == -1) sudo_fatal("dup3");
sudo_fatal("fcntl");
}
if (io_fds[SFD_STDOUT] != STDOUT_FILENO) {
if (dup2(io_fds[SFD_STDOUT], STDOUT_FILENO) == -1)
sudo_fatal("dup2");
if (io_fds[SFD_STDOUT] != io_fds[SFD_SLAVE]) if (io_fds[SFD_STDOUT] != io_fds[SFD_SLAVE])
close(io_fds[SFD_STDOUT]); close(io_fds[SFD_STDOUT]);
} else { if (dup3(io_fds[SFD_STDERR], STDERR_FILENO, 0) == -1)
if (fcntl(io_fds[SFD_STDOUT], F_SETFD, 0) == -1) sudo_fatal("dup3");
sudo_fatal("fcntl");
}
if (io_fds[SFD_STDERR] != STDERR_FILENO) {
if (dup2(io_fds[SFD_STDERR], STDERR_FILENO) == -1)
sudo_fatal("dup2");
if (io_fds[SFD_STDERR] != io_fds[SFD_SLAVE]) if (io_fds[SFD_STDERR] != io_fds[SFD_SLAVE])
close(io_fds[SFD_STDERR]); close(io_fds[SFD_STDERR]);
} else {
if (fcntl(io_fds[SFD_STDERR], F_SETFD, 0) == -1)
sudo_fatal("fcntl");
}
/* Wait for parent to grant us the tty if we are foreground. */ /* Wait for parent to grant us the tty if we are foreground. */
if (foreground && !ISSET(details->flags, CD_EXEC_BG)) { if (foreground && !ISSET(details->flags, CD_EXEC_BG)) {

View File

@@ -227,6 +227,8 @@ relabel_tty(const char *ttyn, int ptyfd)
} }
if (ptyfd != -1) { if (ptyfd != -1) {
int oflags, flags = 0;
/* Reopen pty that was relabeled, std{in,out,err} are reset later. */ /* Reopen pty that was relabeled, std{in,out,err} are reset later. */
se_state.ttyfd = open(ttyn, O_RDWR|O_NOCTTY, 0); se_state.ttyfd = open(ttyn, O_RDWR|O_NOCTTY, 0);
if (se_state.ttyfd == -1 || fstat(se_state.ttyfd, &sb) == -1) { if (se_state.ttyfd == -1 || fstat(se_state.ttyfd, &sb) == -1) {
@@ -238,8 +240,21 @@ relabel_tty(const char *ttyn, int ptyfd)
ttyn); ttyn);
goto bad; goto bad;
} }
if (dup2(se_state.ttyfd, ptyfd) == -1) { /* Preserve O_NONBLOCK and the close-on-exec flags. */
sudo_warn("dup2"); if ((oflags = fcntl(ptyfd, F_GETFL)) == -1) {
sudo_warn("F_GETFL");
goto bad;
}
if (ISSET(oflags, O_NONBLOCK))
flags |= O_NONBLOCK;
if ((oflags = fcntl(ptyfd, F_GETFD)) == -1) {
sudo_warn("F_GETFD");
goto bad;
}
if (ISSET(oflags, FD_CLOEXEC))
flags |= O_CLOEXEC;
if (dup3(se_state.ttyfd, ptyfd, flags) == -1) {
sudo_warn("dup3");
goto bad; goto bad;
} }
} else { } else {

View File

@@ -311,18 +311,11 @@ sudo_askpass(const char *askpass, const char *prompt)
sudo_fatal(U_("unable to fork")); sudo_fatal(U_("unable to fork"));
if (child == 0) { if (child == 0) {
/* child, set stdout to write side of the pipe or clear FD_CLOEXEC */ /* child, set stdout to write side of the pipe */
if (pfd[1] == STDOUT_FILENO) { if (dup3(pfd[1], STDOUT_FILENO, 0) == -1) {
if (fcntl(pfd[1], F_SETFD, 0) == -1) { sudo_warn("dup3");
sudo_warn("fcntl");
_exit(255); _exit(255);
} }
} else {
if (dup2(pfd[1], STDOUT_FILENO) == -1) {
sudo_warn("dup2");
_exit(255);
}
}
if (setuid(ROOT_UID) == -1) if (setuid(ROOT_UID) == -1)
sudo_warn("setuid(%d)", ROOT_UID); sudo_warn("setuid(%d)", ROOT_UID);
/* Close fds before uid change to prevent prlimit sabotage on Linux. */ /* Close fds before uid change to prevent prlimit sabotage on Linux. */