Module Name:    src
Committed By:   kre
Date:           Tue Sep 14 14:49:39 UTC 2021

Modified Files:
        src/bin/sh: parser.c redir.c

Log Message:
Deal with some issues where fds intended only for internal use
by the shell were available for manipulation by scripts (or the user).
These issues were reported by Jan Schaumann on netbsd-users.

The first allows the user to reference sh internal fds, and is
a simple fix - any sh internal fd is simply treated as if it were closed
when referenced by the script.  These fds can be discovered by
examining /proc/N/fd so it is not difficult for a script to discover
which fd it should attempt to access.

The second allows the user to reference a user level fd which is
one that is normally available to it, but at a point where it should
no longer be visible (when that fd has been redirected, for a built
in command, so the original fd needs to be saved so it can be restored,
the saving fd should not be accessible).   It is not as easy for the
script to determine which fd to attempt here, as the relevant one
exists only during the lifetime of a built-in command (and similar),
but there are ways in some cases (aside from looking at /proc from
another process).

Fix this one by watching which fds the user script is attempting
to use, and avoid using those as temporary fds.   This is possible in
this case as we know what command is being run, before we need to
save the fds it uses.   That's different from the earlier case where
when the shell allocates its fds we have no idea what it might
reference later.

Also clean up a couple of other minor code issues (NFC intended) that
I noticed while here...


To generate a diff of this commit:
cvs rdiff -u -r1.172 -r1.173 src/bin/sh/parser.c
cvs rdiff -u -r1.66 -r1.67 src/bin/sh/redir.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/bin/sh/parser.c
diff -u src/bin/sh/parser.c:1.172 src/bin/sh/parser.c:1.173
--- src/bin/sh/parser.c:1.172	Thu Sep  9 01:14:04 2021
+++ src/bin/sh/parser.c	Tue Sep 14 14:49:39 2021
@@ -1,4 +1,4 @@
-/*	$NetBSD: parser.c,v 1.172 2021/09/09 01:14:04 kre Exp $	*/
+/*	$NetBSD: parser.c,v 1.173 2021/09/14 14:49:39 kre Exp $	*/
 
 /*-
  * Copyright (c) 1991, 1993
@@ -37,7 +37,7 @@
 #if 0
 static char sccsid[] = "@(#)parser.c	8.7 (Berkeley) 5/16/95";
 #else
-__RCSID("$NetBSD: parser.c,v 1.172 2021/09/09 01:14:04 kre Exp $");
+__RCSID("$NetBSD: parser.c,v 1.173 2021/09/14 14:49:39 kre Exp $");
 #endif
 #endif /* not lint */
 
@@ -55,6 +55,7 @@ __RCSID("$NetBSD: parser.c,v 1.172 2021/
 #include "options.h"
 #include "input.h"
 #include "output.h"
+#include "redir.h"	/* defines max_user_fd */
 #include "var.h"
 #include "error.h"
 #include "memalloc.h"
@@ -756,6 +757,8 @@ fixredir(union node *n, const char *text
 		else
 			n->ndup.vname = makeword(startlinno - elided_nl);
 	}
+	if (n->ndup.dupfd > max_user_fd)
+		max_user_fd = n->ndup.dupfd;
 }
 
 
@@ -1590,6 +1593,8 @@ parseredir(const char *out,  int c)
 	np->nfile.fd = fd;	/* do this again later with updated fd */
 	if (fd != np->nfile.fd)
 		error("file descriptor (%d) out of range", fd);
+	if (fd > max_user_fd)
+		max_user_fd = fd;
 
 	VTRACE(DBG_LEXER, ("parseredir after '%s%c' ", out, c));
 	if (c == '>') {

Index: src/bin/sh/redir.c
diff -u src/bin/sh/redir.c:1.66 src/bin/sh/redir.c:1.67
--- src/bin/sh/redir.c:1.66	Fri Mar  1 06:15:01 2019
+++ src/bin/sh/redir.c	Tue Sep 14 14:49:39 2021
@@ -1,4 +1,4 @@
-/*	$NetBSD: redir.c,v 1.66 2019/03/01 06:15:01 kre Exp $	*/
+/*	$NetBSD: redir.c,v 1.67 2021/09/14 14:49:39 kre Exp $	*/
 
 /*-
  * Copyright (c) 1991, 1993
@@ -37,7 +37,7 @@
 #if 0
 static char sccsid[] = "@(#)redir.c	8.2 (Berkeley) 5/4/95";
 #else
-__RCSID("$NetBSD: redir.c,v 1.66 2019/03/01 06:15:01 kre Exp $");
+__RCSID("$NetBSD: redir.c,v 1.67 2021/09/14 14:49:39 kre Exp $");
 #endif
 #endif /* not lint */
 
@@ -237,10 +237,14 @@ redirect(union node *redir, int flags)
 		}
 
 		if ((flags & REDIR_PUSH) && !is_renamed(sv->renamed, fd)) {
+			int bigfd;
+
 			INTOFF;
 			if (big_sh_fd < 10)
 				find_big_fd();
-			if ((i = fcntl(fd, F_DUPFD, big_sh_fd)) == -1) {
+			if ((bigfd = big_sh_fd) < max_user_fd)
+				bigfd = max_user_fd;
+			if ((i = fcntl(fd, F_DUPFD, bigfd + 1)) == -1) {
 				switch (errno) {
 				case EBADF:
 					i = CLOSED;
@@ -253,8 +257,7 @@ redirect(union node *redir, int flags)
 						break;
 					/* FALLTHRU */
 				default:
-					i = errno;
-					error("%d: %s", fd, strerror(i));
+					error("%d: %s", fd, strerror(errno));
 					/* NOTREACHED */
 				}
 			}
@@ -350,6 +353,9 @@ openredirect(union node *redir, char mem
 	case NTOFD:
 	case NFROMFD:
 		if (redir->ndup.dupfd >= 0) {	/* if not ">&-" */
+			if (sh_fd(redir->ndup.dupfd) != NULL)
+				error("Redirect (from %d to %d) failed: %s",
+				    redir->ndup.dupfd, fd, strerror(EBADF));
 			if (fd < 10 && redir->ndup.dupfd < 10 &&
 			    memory[redir->ndup.dupfd])
 				memory[fd] = 1;
@@ -684,7 +690,7 @@ renumber_sh_fd(struct shell_fds *fp)
 		find_big_fd();
 
 	to = fcntl(fp->fd, F_DUPFD_CLOEXEC, big_sh_fd);
-	if (to == -1)
+	if (to == -1 && big_sh_fd >= 22)
 		to = fcntl(fp->fd, F_DUPFD_CLOEXEC, big_sh_fd/2);
 	if (to == -1)
 		to = fcntl(fp->fd, F_DUPFD_CLOEXEC, fp->fd + 1);
@@ -828,8 +834,7 @@ getflags(int fd, int p)
 	if (sh_fd(fd) != NULL) {
 		if (!p)
 			return -1;
-		error("Can't get status for fd=%d (%s)", fd,
-		    "Bad file descriptor");			/*XXX*/
+		error("Can't get status for fd=%d (%s)", fd, strerror(EBADF));
 	}
 
 	if ((c = fcntl(fd, F_GETFD)) == -1) {

Reply via email to