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) {