From 0cf7b1d928acaaddd4eaa28c6a22f7bd6457b379 Mon Sep 17 00:00:00 2001

From: Bernhard Voelker <mail@bernhard-voelker.de>

Date: Fri, 01 Aug 2014 00:07:33 +0000

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

* 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

---

diff --git a/doc/coreutils.texi b/doc/coreutils.texi

index 96f0781..7c86719 100644

--- a/doc/coreutils.texi

+++ b/doc/coreutils.texi

@@ -16113,7 +16113,10 @@ On many systems, only the super-user can do this.@footnote{However,

 some systems (e.g., FreeBSD) can be configured to allow certain regular

 users to use the @code{chroot} system call, and hence to run this program.

 Also, on Cygwin, anyone can run the @command{chroot} command, because the

-underlying function is non-privileged due to lack of support in MS-Windows.}

+underlying function is non-privileged due to lack of support in MS-Windows.

+Furthermore, the @command{chroot} command avoids the @code{chroot} system call

+when @var{newroot} is identical to the old @file{/} directory for consistency

+with systems where this is allowed for non-privileged users.}.

 Synopses:

 @example

@@ -16123,10 +16126,11 @@ chroot @var{option}

 Ordinarily, file names are looked up starting at the root of the

 directory structure, i.e., @file{/}.  @command{chroot} changes the root to

-the directory @var{newroot} (which must exist) and then runs

-@var{command} with optional @var{args}.  If @var{command} is not

-specified, the default is the value of the @env{SHELL} environment

-variable or @command{/bin/sh} if not set, invoked with the @option{-i} option.

+the directory @var{newroot} (which must exist), then changes the working

+directory to @file{/}, and finally runs @var{command} with optional @var{args}.

+If @var{command} is not specified, the default is the value of the @env{SHELL}

+environment variable or @command{/bin/sh} if not set, invoked with the

+@option{-i} option.

 @var{command} must not be a special built-in utility

 (@pxref{Special built-in utilities}).

@@ -16135,6 +16139,14 @@ Options must precede operands.

 @table @samp

+@item --groups=@var{groups}

+@opindex --groups

+Use this option to override the supplementary @var{groups} to be

+used by the new process.

+The items in the list (names or numeric IDs) must be separated by commas.

+Use @samp{--groups=''} to disable the supplementary group look-up

+implicit in the @option{--userspec} option.

+

 @item --userspec=@var{user}[:@var{group}]

 @opindex --userspec

 By default, @var{command} is run with the same credentials

@@ -16145,13 +16157,13 @@ If a @var{user} is specified then the supplementary groups

 are set according to the system defined list for that user,

 unless overridden with the @option{--groups} option.

-@item --groups=@var{groups}

-@opindex --groups

-Use this option to override the supplementary @var{groups} to be

-used by the new process.

-The items in the list (names or numeric IDs) must be separated by commas.

-Use @samp{--groups=''} to disable the supplementary group look-up

-implicit in the @option{--userspec} option.

+@item --skip-chdir

+@opindex --skip-chdir

+Use this option to not change the working directory to @file{/} after changing

+the root directory to @var{newroot}, i.e., inside the chroot.

+This option is only permitted when @var{newroot} is the old @file{/} directory,

+and therefore is mostly useful together with the @option{--groups} and

+@option{--userspec} options to retain the previous working directory.

 @end table

diff --git a/init.cfg b/init.cfg

index 725ee12..032646b 100644

--- a/init.cfg

+++ b/init.cfg

@@ -400,7 +400,8 @@ nonroot_has_perm_()

   require_built_ chroot

   local rm_version=$(

-    chroot --user=$NON_ROOT_USERNAME / env PATH="$PATH" rm --version |

+    chroot --skip-chdir --user=$NON_ROOT_USERNAME / env PATH="$PATH" \

+      rm --version |

     sed -n '1s/.* //p'

   )

   case ":$rm_version:" in

diff --git a/src/chroot.c b/src/chroot.c

index 6c2d63f..418ea67 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.  */

diff --git a/tests/cp/preserve-gid.sh b/tests/cp/preserve-gid.sh

index f141ac1..5499c2e 100755

--- a/tests/cp/preserve-gid.sh

+++ b/tests/cp/preserve-gid.sh

@@ -117,7 +117,8 @@ t1() {

   u=$1; shift

   g=$1; shift

   t0 "$f" "$u" "$g" \

-      chroot --user=+$nameless_uid:+$nameless_gid1 \

+      chroot --skip-chdir \

+             --user=+$nameless_uid:+$nameless_gid1 \

              --groups="+$nameless_gid1,+$nameless_gid2" \

         / env PATH="$tmp_path" "$@"

 }

diff --git a/tests/cp/special-bits.sh b/tests/cp/special-bits.sh

index a55eea2..1402819 100755

--- a/tests/cp/special-bits.sh

+++ b/tests/cp/special-bits.sh

@@ -42,7 +42,8 @@ set _ $(ls -l b); shift; p1=$1

 set _ $(ls -l b2); shift; p2=$1

 test $p1 = $p2 || fail=1

-chroot --user=$NON_ROOT_USERNAME / env PATH="$PATH" cp -p c c2 || fail=1

+chroot --skip-chdir --user=$NON_ROOT_USERNAME / env PATH="$PATH" cp -p c c2 \

+  || fail=1

 set _ $(ls -l c); shift; p1=$1

 set _ $(ls -l c2); shift; p2=$1

 test $p1 = $p2 && fail=1

diff --git a/tests/id/setgid.sh b/tests/id/setgid.sh

index 6d9d74f..019418a 100755

--- a/tests/id/setgid.sh

+++ b/tests/id/setgid.sh

@@ -27,14 +27,14 @@ echo $gp1 > exp || framework_failure_

 # With coreutils-8.16 and earlier, id -G would print both:

 #  $gp1 $NON_ROOT_GID

-chroot --user=$NON_ROOT_USERNAME:+$gp1 --groups='' / env PATH="$PATH" \

-  id -G > out || fail=1

+chroot --skip-chdir --user=$NON_ROOT_USERNAME:+$gp1 --groups='' / \

+  env PATH="$PATH" id -G > out || fail=1

 compare exp out || fail=1

 # With coreutils-8.22 and earlier, id would erroneously print

 #  groups=$NON_ROOT_GID

-chroot --user=$NON_ROOT_USERNAME:+$gp1 --groups='' / env PATH="$PATH" \

-  id > out || fail=1

+chroot --skip-chdir --user=$NON_ROOT_USERNAME:+$gp1 --groups='' / \

+  env PATH="$PATH" id > out || fail=1

 grep -F "groups=$gp1" out || { cat out; fail=1; }

 Exit $fail

diff --git a/tests/misc/chroot-fail.sh b/tests/misc/chroot-fail.sh

index a84826f..82ae23c 100755

--- a/tests/misc/chroot-fail.sh

+++ b/tests/misc/chroot-fail.sh

@@ -30,7 +30,7 @@ chroot --- / true # unknown option

 test $? = 125 || fail=1

 # Note chroot("/") succeeds for non-root users on some systems, but not all,

-# however we avoid the chroot() with "/" to have common behvavior.

+# however we avoid the chroot() with "/" to have common behavior.

 chroot / sh -c 'exit 2' # exit status propagation

 test $? = 2 || fail=1

 chroot / . # invalid command

@@ -38,10 +38,25 @@ test $? = 126 || fail=1

 chroot / no_such # no such command

 test $? = 127 || fail=1

-# Ensure we don't chdir("/") when not changing root

-# to allow only changing user ids for a command.

-for dir in '/' '/.' '/../'; do

+# Ensure that --skip-chdir fails with a non-"/" argument.

+cat <<\EOF > exp || framework_failure_

+chroot: option --skip-chdir only permitted if NEWROOT is old '/'

+Try 'chroot --help' for more information.

+EOF

+chroot --skip-chdir . env pwd >out 2>err && fail=1

+compare /dev/null out || fail=1

+compare exp err || fail=1

+

+# Ensure we don't chroot("/") when NEWROOT is old "/".

+ln -s / isroot || framework_failure_

+for dir in '/' '/.' '/../' isroot; do

+  # Verify that chroot(1) succeeds and performs chdir("/")

+  # (chroot(1) of coreutils-8.23 failed to run the latter).

   curdir=$(chroot "$dir" env pwd) || fail=1

+  test "$curdir" = '/' || fail=1

+

+  # Test the "--skip-chdir" option.

+  curdir=$(chroot --skip-chdir "$dir" env pwd) || fail=1

   test "$curdir" = '/' && fail=1

 done

diff --git a/tests/misc/truncate-owned-by-other.sh b/tests/misc/truncate-owned-by-other.sh

index e70badb..f65439e 100755

--- a/tests/misc/truncate-owned-by-other.sh

+++ b/tests/misc/truncate-owned-by-other.sh

@@ -29,7 +29,7 @@ chmod g+w root-owned

 # Ensure that the current directory is searchable by $NON_ROOT_USERNAME.

 chmod g+x .

-chroot --user=$NON_ROOT_USERNAME / env PATH="$PATH" \

+chroot --skip-chdir --user=$NON_ROOT_USERNAME / env PATH="$PATH" \

   truncate -s0 root-owned || fail=1

 Exit $fail

diff --git a/tests/mv/sticky-to-xpart.sh b/tests/mv/sticky-to-xpart.sh

index e0c99e9..6c1f6e8 100755

--- a/tests/mv/sticky-to-xpart.sh

+++ b/tests/mv/sticky-to-xpart.sh

@@ -42,7 +42,8 @@ chmod go+x . || framework_failure_

 # Ensure that $NON_ROOT_USERNAME can access the required version of mv.

 version=$(

-  chroot --user=$NON_ROOT_USERNAME / env PATH="$PATH" mv --version |

+  chroot --skip-chdir --user=$NON_ROOT_USERNAME / env PATH="$PATH" \

+    mv --version |

   sed -n '1s/.* //p'

 )

 case $version in

@@ -50,7 +51,7 @@ case $version in

   *) skip_ "cannot access just-built mv as user $NON_ROOT_USERNAME";;

 esac

-chroot --user=$NON_ROOT_USERNAME / env PATH="$PATH" \

+chroot --skip-chdir --user=$NON_ROOT_USERNAME / env PATH="$PATH" \

   mv t/root-owned "$other_partition_tmpdir" 2> out-t && fail=1

 # On some systems, we get 'Not owner'.  Convert it.

diff --git a/tests/rm/fail-2eperm.sh b/tests/rm/fail-2eperm.sh

index 6e8ce9b..c324037 100755

--- a/tests/rm/fail-2eperm.sh

+++ b/tests/rm/fail-2eperm.sh

@@ -32,14 +32,16 @@ touch a/b || framework_failure_

 # Try to ensure that $NON_ROOT_USERNAME can access

 # the required version of rm.

 rm_version=$(

-  chroot --user=$NON_ROOT_USERNAME / env PATH="$PATH" rm --version |

+  chroot --skip-chdir --user=$NON_ROOT_USERNAME / env PATH="$PATH" \

+    rm --version |

   sed -n '1s/.* //p'

 )

 case $rm_version in

   $PACKAGE_VERSION) ;;

   *) skip_ "cannot access just-built rm as user $NON_ROOT_USERNAME";;

 esac

-chroot --user=$NON_ROOT_USERNAME / env PATH="$PATH" rm -rf a 2> out-t && fail=1

+chroot --skip-chdir --user=$NON_ROOT_USERNAME / \

+  env PATH="$PATH" rm -rf a 2> out-t && fail=1

 # On some systems, we get 'Not owner'.  Convert it.

 # On other systems (HPUX), we get 'Permission denied'.  Convert it, too.

diff --git a/tests/rm/no-give-up.sh b/tests/rm/no-give-up.sh

index 41070c9..958f9e8 100755

--- a/tests/rm/no-give-up.sh

+++ b/tests/rm/no-give-up.sh

@@ -30,7 +30,7 @@ chmod go=x . || framework_failure_

 # This must fail, since '.' is not writable by $NON_ROOT_USERNAME.

-chroot --user=$NON_ROOT_USERNAME / env PATH="$PATH" \

+chroot --skip-chdir --user=$NON_ROOT_USERNAME / env PATH="$PATH" \

   rm -rf d 2>/dev/null && fail=1

 # d must remain.

diff --git a/tests/touch/now-owned-by-other.sh b/tests/touch/now-owned-by-other.sh

index d01097e..018ef11 100755

--- a/tests/touch/now-owned-by-other.sh

+++ b/tests/touch/now-owned-by-other.sh

@@ -28,7 +28,7 @@ chmod g+w root-owned

 # Ensure that the current directory is searchable by $NON_ROOT_USERNAME.

 chmod g+x .

-chroot --user=$NON_ROOT_USERNAME / env PATH="$PATH" \

+chroot --skip-chdir --user=$NON_ROOT_USERNAME / env PATH="$PATH" \

   touch -d now root-owned || fail=1

 Exit $fail

--

cgit v0.9.0.2