From 0cf7b1d928acaaddd4eaa28c6a22f7bd6457b379 Mon Sep 17 00:00:00 2001 From: Bernhard Voelker Date: Fri, 1 Aug 2014 02:07:33 +0200 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. * 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 --- src/chroot.c | 38 ++++++++++++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 8 deletions(-) (limited to 'src') 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. */ -- cgit v1.2.3-54-g00ecf