From aa5f0f5745e4e7392c078d1fe28825c941f52f7c Mon Sep 17 00:00:00 2001 From: Tavian Barnes Date: Thu, 31 Jan 2019 23:46:56 -0500 Subject: main: Fix closed standard stream handling bfs >&- should complain about a missing file descriptor, rather than silently succeeding. --- main.c | 40 ++++++++++++++++++++++++--------------- tests.sh | 29 ++++++++++++++++++++++++++++ tests/test_closed_stdin.out | 19 +++++++++++++++++++ tests/test_ok_closed_stdin.out | 0 tests/test_okdir_closed_stdin.out | 0 util.c | 10 +++------- 6 files changed, 76 insertions(+), 22 deletions(-) create mode 100644 tests/test_closed_stdin.out create mode 100644 tests/test_ok_closed_stdin.out create mode 100644 tests/test_okdir_closed_stdin.out diff --git a/main.c b/main.c index 2734aaf..3c19092 100644 --- a/main.c +++ b/main.c @@ -1,6 +1,6 @@ /**************************************************************************** * bfs * - * Copyright (C) 2015-2017 Tavian Barnes * + * Copyright (C) 2015-2019 Tavian Barnes * * * * Permission to use, copy, modify, and/or distribute this software for any * * purpose with or without fee is hereby granted. * @@ -24,16 +24,31 @@ #include /** - * Ensure that a file descriptor is open. + * Make sure the standard streams std{in,out,err} are open. If they are not, + * future open() calls may use those file descriptors, and std{in,out,err} will + * use them unintentionally. */ -static int ensure_fd_open(int fd, int flags) { - if (isopen(fd)) { - return 0; - } else if (redirect(fd, "/dev/null", flags) >= 0) { - return 0; - } else { +static int open_std_streams() { +#ifdef O_PATH + const int inflags = O_PATH, outflags = O_PATH; +#else + // These are intentionally backwards so that bfs >&- still fails with EBADF + const int inflags = O_WRONLY, outflags = O_RDONLY; +#endif + + if (!isopen(STDERR_FILENO) && redirect(STDERR_FILENO, "/dev/null", outflags) < 0) { + return -1; + } + if (!isopen(STDOUT_FILENO) && redirect(STDOUT_FILENO, "/dev/null", outflags) < 0) { + perror("redirect()"); + return -1; + } + if (!isopen(STDIN_FILENO) && redirect(STDIN_FILENO, "/dev/null", inflags) < 0) { + perror("redirect()"); return -1; } + + return 0; } /** @@ -42,13 +57,8 @@ static int ensure_fd_open(int fd, int flags) { int main(int argc, char *argv[]) { int ret = EXIT_FAILURE; - if (ensure_fd_open(STDIN_FILENO, O_RDONLY) != 0) { - goto done; - } - if (ensure_fd_open(STDOUT_FILENO, O_WRONLY) != 0) { - goto done; - } - if (ensure_fd_open(STDERR_FILENO, O_WRONLY) != 0) { + // Make sure the standard streams are open + if (open_std_streams() != 0) { goto done; } diff --git a/tests.sh b/tests.sh index 775a254..302a4c2 100755 --- a/tests.sh +++ b/tests.sh @@ -290,6 +290,11 @@ posix_tests=( test_user_name test_user_id + # Closed file descriptors + test_closed_stdin + test_closed_stdout + test_closed_stderr + # PATH_MAX handling test_deep @@ -385,8 +390,10 @@ bsd_tests=( test_nouser test_ok_stdin + test_ok_closed_stdin test_okdir_stdin + test_okdir_closed_stdin test_perm_000_plus test_perm_222_plus @@ -525,8 +532,10 @@ gnu_tests=( test_nouser + test_ok_closed_stdin test_ok_nothing + test_okdir_closed_stdin test_okdir_plus_semicolon test_perm_000_slash @@ -1929,6 +1938,26 @@ function test_fprint_error() { fi } +function test_closed_stdin() { + bfs_diff basic <&- +} + +function test_ok_closed_stdin() { + bfs_diff basic -ok echo \; <&- 2>/dev/null +} + +function test_okdir_closed_stdin() { + bfs_diff basic -okdir echo {} \; <&- 2>/dev/null +} + +function test_closed_stdout() { + ! invoke_bfs basic >&- 2>/dev/null +} + +function test_closed_stderr() { + ! invoke_bfs basic >&- 2>&- +} + if [ -t 1 -a ! "$VERBOSE" ]; then in_place=yes fi diff --git a/tests/test_closed_stdin.out b/tests/test_closed_stdin.out new file mode 100644 index 0000000..bb3cd8d --- /dev/null +++ b/tests/test_closed_stdin.out @@ -0,0 +1,19 @@ +basic +basic/a +basic/b +basic/c +basic/e +basic/g +basic/i +basic/j +basic/k +basic/l +basic/c/d +basic/e/f +basic/g/h +basic/j/foo +basic/k/foo +basic/l/foo +basic/k/foo/bar +basic/l/foo/bar +basic/l/foo/bar/baz diff --git a/tests/test_ok_closed_stdin.out b/tests/test_ok_closed_stdin.out new file mode 100644 index 0000000..e69de29 diff --git a/tests/test_okdir_closed_stdin.out b/tests/test_okdir_closed_stdin.out new file mode 100644 index 0000000..e69de29 diff --git a/util.c b/util.c index b708527..1e5ebdb 100644 --- a/util.c +++ b/util.c @@ -85,8 +85,6 @@ bool isopen(int fd) { } int redirect(int fd, const char *path, int flags, ...) { - close(fd); - mode_t mode = 0; if (flags & O_CREAT) { va_list args; @@ -102,11 +100,9 @@ int redirect(int fd, const char *path, int flags, ...) { int ret = open(path, flags, mode); if (ret >= 0 && ret != fd) { - int other = ret; - ret = dup2(other, fd); - if (close(other) != 0) { - ret = -1; - } + int orig = ret; + ret = dup2(orig, fd); + close(orig); } return ret; -- cgit v1.2.3