[src/trunk]: src/bin/sh Deal with some issues where fds intended only for int...

[kre <kre%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/fdd68c7e0e34
branches:  trunk
changeset: 986225:fdd68c7e0e34
user:      kre <kre%NetBSD.org@localhost>
date:      Tue Sep 14 14:49:39 2021 +0000

description:
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...

diffstat:

 bin/sh/parser.c |   9 +++++++--
 bin/sh/redir.c  |  21 +++++++++++++--------
 2 files changed, 20 insertions(+), 10 deletions(-)

diffs (117 lines):

diff -r 87eff03fca39 -r fdd68c7e0e34 bin/sh/parser.c
--- a/bin/sh/parser.c   Tue Sep 14 01:33:19 2021 +0000
+++ b/bin/sh/parser.c   Tue Sep 14 14:49:39 2021 +0000
@@ -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 @@
 #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 @@
                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 @@
        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 == '>') {
diff -r 87eff03fca39 -r fdd68c7e0e34 bin/sh/redir.c
--- a/bin/sh/redir.c    Tue Sep 14 01:33:19 2021 +0000
+++ b/bin/sh/redir.c    Tue Sep 14 14:49:39 2021 +0000
@@ -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 @@
                }
 
                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 @@
                                                break;
                                        /* FALLTHRU */
                                default:
-                                       i = errno;
-                                       error("%d: %s", fd, strerror(i));
+                                       error("%d: %s", fd, strerror(errno));
                                        /* NOTREACHED */
                                }
                        }
@@ -350,6 +353,9 @@
        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 @@
                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 @@
        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) {