main: Fix closed standard stream handling

bfs >&- should complain about a missing file descriptor, rather than
silently succeeding.

6 files changed, 76 insertions, 22 deletions

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 <tavianator@tavianator.com>        *
+ * Copyright (C) 2015-2019 Tavian Barnes <tavianator@tavianator.com>        *
  *                                                                          *
  * 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 <unistd.h>
 
 /**
- * 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
--- /dev/null
+++ b/tests/test_ok_closed_stdin.out
diff --git a/tests/test_okdir_closed_stdin.out b/tests/test_okdir_closed_stdin.out
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/tests/test_okdir_closed_stdin.out
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;