chroot: perform chdir("/") again unless new --skip-chdir is specified

Since commit v8.22-94-g99960ee, chroot(1) skips the chroot(2) syscall
for "/" arguments (and synonyms).  The problem is that it also skips
the following chdir("/") call in that case.  The latter breaks existing
scripts which expect "/" to be the working directory inside the chroot.
While the first part of the change - i.e., skipping chroot("/") - is
okay for consistency with systems where it might succeed for a non-root
user, the second part might be malicious, e.g.

  cd /home/user && chroot '/' bin/foo

In the "best" case, chroot(1) could not execute 'bin/foo' with ENOENT,
but in the worst case, chroot(1) would execute '/home/user/bin/foo' in
the case that exists - instead of '/bin/foo'.

Revert that second part of the patch, i.e., perform the chdir("/)
in the common case again - unless the new --skip-chdir option is
specified.  Restrict this new option to the case of "/" arguments.

* src/chroot.c (SKIP_CHDIR): Add enum.
(long_opts): Add entry for the new --skip-chdir option.
(usage): Add --skip-chdir option, and while at it, move the other
to options into alphabetical order.
(main): Accept the above new option, allowing it only in the case
when NEWROOT is the old "/".
Move down the chdir() call after the if-clause to ensure it is
run in any case - unless --skip-chdir is specified.
Add a 'newroot' variable for the new root directory as it is used
in a couple of places now.
* tests/misc/chroot-fail.sh: Invert the last tests which check the
working directory of the execvp()ed program when a "/"-like
argument was passed: now expect it to be "/" - unless --skip-chdir
is given.
* doc/coreutils.texi (chroot invocation): Document the new option.
Document that chroot(1) usually calls chdir("/") unless the new
--skip-chdir option is specified.  Sort options.
* NEWS (Changes in behavior): Mention the fix.
(New features): Mention the new option.
* init.cfg (nonroot_has_perm_): Add chroot's new --skip-chdir option.
* tests/cp/preserve-gid.sh (t1): Likewise.
* tests/cp/special-bits.sh: Likewise.
* tests/id/setgid.sh: Likewise.
* tests/misc/truncate-owned-by-other.sh: Likewise.
* tests/mv/sticky-to-xpart.sh: Likewise.
* tests/rm/fail-2eperm.sh: Likewise.
* tests/rm/no-give-up.sh: Likewise.
* tests/touch/now-owned-by-other.sh: Likewise.

Reported by Andreas Schwab in http://bugs.gnu.org/18062

1 files changed, 30 insertions, 8 deletions

diff --git a/src/chroot.c b/src/chroot.c
index 6c2d63f55..418ea673f 100644
--- a/src/chroot.c
+++ b/src/chroot.c
@@ -49,13 +49,15 @@ static inline bool gid_unset (gid_t gid) { return gid == (gid_t) -1; }
 enum
 {
   GROUPS = UCHAR_MAX + 1,
-  USERSPEC
+  USERSPEC,
+  SKIP_CHDIR
 };
 
 static struct option const long_opts[] =
 {
   {"groups", required_argument, NULL, GROUPS},
   {"userspec", required_argument, NULL, USERSPEC},
+  {"skip-chdir", no_argument, NULL, SKIP_CHDIR},
   {GETOPT_HELP_OPTION_DECL},
   {GETOPT_VERSION_OPTION_DECL},
   {NULL, 0, NULL, 0}
@@ -194,9 +196,14 @@ Run COMMAND with root directory set to NEWROOT.\n\
 "), stdout);
 
       fputs (_("\
-  --userspec=USER:GROUP  specify user and group (ID or name) to use\n\
   --groups=G_LIST        specify supplementary groups as g1,g2,..,gN\n\
 "), stdout);
+      fputs (_("\
+  --userspec=USER:GROUP  specify user and group (ID or name) to use\n\
+"), stdout);
+      printf (_("\
+  --skip-chdir           do not change working directory to %s\n\
+"), quote ("/"));
 
       fputs (HELP_OPTION_DESCRIPTION, stdout);
       fputs (VERSION_OPTION_DESCRIPTION, stdout);
@@ -218,6 +225,7 @@ main (int argc, char **argv)
   char *userspec = NULL;
   char const *username = NULL;
   char const *groups = NULL;
+  bool skip_chdir = false;
 
   /* Parsed user and group IDs.  */
   uid_t uid = -1;
@@ -254,6 +262,10 @@ main (int argc, char **argv)
           groups = optarg;
           break;
 
+        case SKIP_CHDIR:
+          skip_chdir = true;
+          break;
+
         case_GETOPT_HELP_CHAR;
 
         case_GETOPT_VERSION_CHAR (PROGRAM_NAME, AUTHORS);
@@ -269,9 +281,19 @@ main (int argc, char **argv)
       usage (EXIT_CANCELED);
     }
 
+  char const *newroot = argv[optind];
+  bool is_oldroot = is_root (newroot);
+
+  if (! is_oldroot && skip_chdir)
+    {
+      error (0, 0, _("option --skip-chdir only permitted if NEWROOT is old %s"),
+             quote ("/"));
+      usage (EXIT_CANCELED);
+    }
+
   /* Only do chroot specific actions if actually changing root.
      The main difference here is that we don't change working dir.  */
-  if (! is_root (argv[optind]))
+  if (! is_oldroot)
     {
       /* We have to look up users and groups twice.
         - First, outside the chroot to load potentially necessary passwd/group
@@ -307,14 +329,14 @@ main (int argc, char **argv)
         }
 #endif
 
-      if (chroot (argv[optind]) != 0)
+      if (chroot (newroot) != 0)
         error (EXIT_CANCELED, errno, _("cannot change root directory to %s"),
-               argv[optind]);
-
-      if (chdir ("/"))
-        error (EXIT_CANCELED, errno, _("cannot chdir to root directory"));
+               newroot);
     }
 
+  if (! skip_chdir && chdir ("/"))
+    error (EXIT_CANCELED, errno, _("cannot chdir to root directory"));
+
   if (argc == optind + 1)
     {
       /* No command.  Run an interactive shell.  */