From 07a69fc3ba717b879218592db685a1c79869cb28 Mon Sep 17 00:00:00 2001 From: =?utf8?q?P=C3=A1draig=20Brady?= Date: Mon, 28 Nov 2016 09:13:39 +0100 Subject: [PATCH] chmod: add support for -h, -H,-L,-P, --dereference options There have been various requests to add -h to avoid following symlinks for security reasons. This wasn't provided previously as chmod(1) already ignored symlinks unless specified on the command line. Note chmod defaults to -H mode rather than the chown default of -P, as usually chown can work directly on symlinks and so defaults to not traversing those specified on the command line. Note FreeBSD chmod does default to -P mode, but we retain the -H mode default also for compatibility with existing chmod behavior. Adding -HLP will allow chmod to disable traversing CLI symlinks to dirs. Adding -h will allow to disable following CLI symlinks to files/dirs, also operating on all symlinks on systems that support that. Adding --dereference will be significant with -H (the default). I.e. symlinks to dirs not recursed, but symlinks are dereferenced. Adding these options will also be consistent with chown(1), chgrp(1), and chmod(1) on other systems. Note since chmod(1) currently ignores symlinks by default, and -h is primarily a mechanism to avoid following symlinks, rather than for operating on the symlink itself, we make -h try to chmod a symlink, but ignore ENOTSUP. In that way we're consistent with chown(1) where it also ignores ENOTSUP for symlinks, and we don't fail when trying to be extra secure with command line params. * doc/coreutils.texi (chmod invocation): Reference the -H,-L,-P descriptions, and adjust the corresponding macros to say the default is -H or -P as appropriate. Add --dereference and -h,--no-dereference descriptions. * man/chmod.x: Adjust discussion of symlink handling. * src/chmod.c (main): Accept new options and set fts flags appropriately. (process_file): Process / dereference symlinks as necessary. * src/system.h (emit_symlink_recurse_options): A new function refactored from chown.c and chmod.c usage(). * tests/chmod/symlinks.sh: New test for the new options. * tests/local.mk: Reference the new test. * NEWS: Mention the new feature. --- NEWS | 4 ++ doc/coreutils.texi | 53 ++++++++++++++++++++---- man/chmod.x | 10 +++-- src/chmod.c | 106 ++++++++++++++++++++++++++++++++++++++++++------ src/chown.c | 14 +------ src/system.h | 18 ++++++++ tests/chmod/symlinks.sh | 87 +++++++++++++++++++++++++++++++++++++++ tests/local.mk | 1 + 8 files changed, 254 insertions(+), 39 deletions(-) create mode 100755 tests/chmod/symlinks.sh diff --git a/NEWS b/NEWS index ef1ad33bd..0f4503cbb 100644 --- a/NEWS +++ b/NEWS @@ -76,6 +76,10 @@ GNU coreutils NEWS -*- outline -*- chgrp now accepts the --from=OWNER:GROUP option to restrict changes to files with matching current OWNER and/or GROUP, as already supported by chown(1). + chmod adds support for -h, -H,-L,-P, and --dereference options, providing + more control over symlink handling. This supports more secure handling of + CLI arguments, and is more consistent with chown, and chmod on other systems. + cp now accepts the --keep-directory-symlink option (like tar), to preserve and follow existing symlinks to directories in the destination. diff --git a/doc/coreutils.texi b/doc/coreutils.texi index 3d2982aa7..ec15a467f 100644 --- a/doc/coreutils.texi +++ b/doc/coreutils.texi @@ -1375,11 +1375,11 @@ other parts of that standard. @cindex symbolic link to directory, controlling traversal of -The following options modify how @command{chown} and @command{chgrp} @c FIXME: note that 'du' has these options, too, but they have slightly @c different meaning. -traverse a hierarchy when the @option{--recursive} (@option{-R}) -option is also specified. +The following options modify how @command {chmod}, @command{chown}, +and @command{chgrp} traverse a hierarchy when +the @option{--recursive} (@option{-R}) option is also specified. If more than one of the following options is specified, only the final one takes effect. These options specify whether processing a symbolic link to a directory @@ -1428,10 +1428,14 @@ possibly allowing the attacker to escalate privileges. @opindex -P @cindex symbolic link to directory, never traverse Do not traverse any symbolic links. +@end macro +@choptP + +@macro choptDefault This is the default if none of @option{-H}, @option{-L}, or @option{-P} is specified. @end macro -@choptP +@choptDefault @end table @@ -11723,6 +11727,7 @@ Recursively change ownership of directories and their contents. @xref{Traversing symlinks}. @choptP +@choptDefault @xref{Traversing symlinks}. @end table @@ -11836,6 +11841,7 @@ Recursively change the group ownership of directories and their contents. @xref{Traversing symlinks}. @choptP +@choptDefault @xref{Traversing symlinks}. @end table @@ -11869,13 +11875,14 @@ chmod [@var{option}]@dots{} @{@var{mode} | --reference=@var{ref_file}@}@c @end example @cindex symbolic links, permissions of -@command{chmod} never changes the permissions of symbolic links, since -the @command{chmod} system call cannot change their permissions. -This is not a problem since the permissions of symbolic links are -never used. However, for each symbolic link listed on the command +@command{chmod} doesn't change the permissions of symbolic links, since +the @command{chmod} system call cannot change their permissions on most systems, +and most systems ignore permissions of symbolic links. +However, for each symbolic link listed on the command line, @command{chmod} changes the permissions of the pointed-to file. In contrast, @command{chmod} ignores symbolic links encountered during -recursive directory traversals. +recursive directory traversals. Options that modify this behavior +are described below. Only a process whose effective user ID matches the user ID of the file, or a process with appropriate privileges, is permitted to change the @@ -11909,6 +11916,23 @@ The program accepts the following options. Also see @ref{Common options}. Verbosely describe the action for each @var{file} whose permissions actually change. +@item --dereference +@opindex --dereference +@cindex symbolic links, changing mode +Do not act on symbolic links themselves but rather on what they point to. +This is the default for command line arguments, but not for +symbolic links encountered when recursing. +@warnOptDerefWithRec + +@item -h +@itemx --no-dereference +@opindex -h +@opindex --no-dereference +@cindex symbolic links, changing mode +Act on symbolic links themselves instead of what they point to. +On systems that do not support this, no diagnostic is issued, +but see @option{--verbose}. + @item -f @itemx --silent @itemx --quiet @@ -11952,6 +11976,17 @@ of the symbolic link, but rather that of the file it refers to. @cindex recursively changing access permissions Recursively change permissions of directories and their contents. +@choptH +@choptDefault +@xref{Traversing symlinks}. + +@choptL +@warnOptDerefWithRec +@xref{Traversing symlinks}. + +@choptP +@xref{Traversing symlinks}. + @end table @exitstatus diff --git a/man/chmod.x b/man/chmod.x index 71de4b920..2954f55ee 100644 --- a/man/chmod.x +++ b/man/chmod.x @@ -60,17 +60,19 @@ file's group, with the same values; and the fourth for other users not in the file's group, with the same values. .PP .B chmod -never changes the permissions of symbolic links; the +doesn't change the permissions of symbolic links; the .B chmod -system call cannot change their permissions. This is not a problem -since the permissions of symbolic links are never used. +system call cannot change their permissions on most systems, +and most systems ignore permissions of symbolic links. However, for each symbolic link listed on the command line, .B chmod changes the permissions of the pointed-to file. In contrast, .B chmod ignores symbolic links encountered during recursive directory -traversals. +traversals. Options that modify this behavior are described in the +.B OPTIONS +section. .SH "SETUID AND SETGID BITS" .B chmod clears the set-group-ID bit of a diff --git a/src/chmod.c b/src/chmod.c index fcd1d3e8f..835b75d9a 100644 --- a/src/chmod.c +++ b/src/chmod.c @@ -74,6 +74,10 @@ static mode_t umask_value; /* If true, change the modes of directories recursively. */ static bool recurse; +/* 1 if --dereference, 0 if --no-dereference, -1 if neither has been + specified. */ +static int dereference = -1; + /* If true, force silence (suppress most of error messages). */ static bool force_silent; @@ -93,7 +97,8 @@ static struct dev_ino *root_dev_ino; non-character as a pseudo short option, starting with CHAR_MAX + 1. */ enum { - NO_PRESERVE_ROOT = CHAR_MAX + 1, + DEREFERENCE_OPTION = CHAR_MAX + 1, + NO_PRESERVE_ROOT, PRESERVE_ROOT, REFERENCE_FILE_OPTION }; @@ -101,7 +106,9 @@ enum static struct option const long_options[] = { {"changes", no_argument, nullptr, 'c'}, + {"dereference", no_argument, nullptr, DEREFERENCE_OPTION}, {"recursive", no_argument, nullptr, 'R'}, + {"no-dereference", no_argument, nullptr, 'h'}, {"no-preserve-root", no_argument, nullptr, NO_PRESERVE_ROOT}, {"preserve-root", no_argument, nullptr, PRESERVE_ROOT}, {"quiet", no_argument, nullptr, 'f'}, @@ -207,6 +214,7 @@ process_file (FTS *fts, FTSENT *ent) const struct stat *file_stats = ent->fts_statp; struct change_status ch = {0}; ch.status = CH_NO_STAT; + struct stat stat_buf; switch (ent->fts_info) { @@ -244,9 +252,30 @@ process_file (FTS *fts, FTSENT *ent) break; case FTS_SLNONE: - if (! force_silent) - error (0, 0, _("cannot operate on dangling symlink %s"), - quoteaf (file_full_name)); + if (dereference) + { + if (! force_silent) + error (0, 0, _("cannot operate on dangling symlink %s"), + quoteaf (file_full_name)); + break; + } + ch.status = CH_NOT_APPLIED; + break; + + case FTS_SL: + if (dereference == 1) + { + if (fstatat (fts->fts_cwd_fd, file, &stat_buf, 0) != 0) + { + if (! force_silent) + error (0, errno, _("cannot dereference %s"), + quoteaf (file_full_name)); + break; + } + + file_stats = &stat_buf; + } + ch.status = CH_NOT_APPLIED; break; case FTS_DC: /* directory that causes cycles */ @@ -272,19 +301,30 @@ process_file (FTS *fts, FTSENT *ent) return false; } - if (ch.status == CH_NOT_APPLIED && ! S_ISLNK (file_stats->st_mode)) + /* With -H (default) or -P, (without -h), avoid operating on symlinks. + With -L, S_ISLNK should be false, and with -RP, dereference is 0. */ + if (ch.status == CH_NOT_APPLIED + && ! (S_ISLNK (file_stats->st_mode) && dereference == -1)) { ch.old_mode = file_stats->st_mode; ch.new_mode = mode_adjust (ch.old_mode, S_ISDIR (ch.old_mode) != 0, umask_value, change, nullptr); - if (chmodat (fts->fts_cwd_fd, file, ch.new_mode) == 0) + /* XXX: Racy if FILE is now replaced with a symlink, which is + a potential security issue with -[H]R. */ + if (fchmodat (fts->fts_cwd_fd, file, ch.new_mode, + dereference ? 0 : AT_SYMLINK_NOFOLLOW) == 0) ch.status = CH_SUCCEEDED; else { - if (! force_silent) - error (0, errno, _("changing permissions of %s"), - quoteaf (file_full_name)); - ch.status = CH_FAILED; + if (! is_ENOTSUP (errno)) + { + if (! force_silent) + error (0, errno, _("changing permissions of %s"), + quoteaf (file_full_name)); + + ch.status = CH_FAILED; + } + /* else treat not supported as not applied. */ } } @@ -389,6 +429,11 @@ With --reference, change the mode of each FILE to that of RFILE.\n\ -v, --verbose output a diagnostic for every file processed\n\ "), stdout); fputs (_("\ + --dereference affect the referent of each symbolic link,\n\ + rather than the symbolic link itself\n\ + -h, --no-dereference affect each symbolic link, rather than the referent\n\ +"), stdout); + fputs (_("\ --no-preserve-root do not treat '/' specially (the default)\n\ --preserve-root fail to operate recursively on '/'\n\ "), stdout); @@ -399,6 +444,7 @@ With --reference, change the mode of each FILE to that of RFILE.\n\ fputs (_("\ -R, --recursive change files and directories recursively\n\ "), stdout); + emit_symlink_recurse_options ("-H"); fputs (HELP_OPTION_DESCRIPTION, stdout); fputs (VERSION_OPTION_DESCRIPTION, stdout); fputs (_("\ @@ -423,6 +469,7 @@ main (int argc, char **argv) bool preserve_root = false; char const *reference_file = nullptr; int c; + int bit_flags = FTS_COMFOLLOW | FTS_PHYSICAL; initialize_main (&argc, &argv); set_program_name (argv[0]); @@ -435,13 +482,35 @@ main (int argc, char **argv) recurse = force_silent = diagnose_surprises = false; while ((c = getopt_long (argc, argv, - ("Rcfvr::w::x::X::s::t::u::g::o::a::,::+::=::" + ("HLPRcfhvr::w::x::X::s::t::u::g::o::a::,::+::=::" "0::1::2::3::4::5::6::7::"), long_options, nullptr)) != -1) { switch (c) { + + case 'H': /* Traverse command-line symlinks-to-directories. */ + bit_flags = FTS_COMFOLLOW | FTS_PHYSICAL; + break; + + case 'L': /* Traverse all symlinks-to-directories. */ + bit_flags = FTS_LOGICAL; + break; + + case 'P': /* Traverse no symlinks-to-directories. */ + bit_flags = FTS_PHYSICAL; + break; + + case 'h': /* --no-dereference: affect symlinks */ + dereference = 0; + break; + + case DEREFERENCE_OPTION: /* --dereference: affect the referent + of each symlink */ + dereference = 1; + break; + case 'r': case 'w': case 'x': @@ -510,6 +579,17 @@ main (int argc, char **argv) } } + if (recurse) + { + if (bit_flags == FTS_PHYSICAL) + { + if (dereference == 1) + error (EXIT_FAILURE, 0, + _("-R --dereference requires either -H or -L")); + dereference = 0; + } + } + if (reference_file) { if (mode) @@ -564,8 +644,8 @@ main (int argc, char **argv) root_dev_ino = nullptr; } - ok = process_files (argv + optind, - FTS_COMFOLLOW | FTS_PHYSICAL | FTS_DEFER_STAT); + bit_flags |= FTS_DEFER_STAT; + ok = process_files (argv + optind, bit_flags); main_exit (ok ? EXIT_SUCCESS : EXIT_FAILURE); } diff --git a/src/chown.c b/src/chown.c index 1262fa7a8..48e0d9c5b 100644 --- a/src/chown.c +++ b/src/chown.c @@ -121,19 +121,7 @@ With --reference, change the group of each FILE to that of RFILE.\n\ fputs (_("\ -R, --recursive operate on files and directories recursively\n\ "), stdout); - fputs (_("\ -\n\ -The following options modify how a hierarchy is traversed when the -R\n\ -option is also specified. If more than one is specified, only the final\n\ -one takes effect.\n\ -\n\ - -H if a command line argument is a symbolic link\n\ - to a directory, traverse it\n\ - -L traverse every symbolic link to a directory\n\ - encountered\n\ - -P do not traverse any symbolic links (default)\n\ -\n\ -"), stdout); + emit_symlink_recurse_options ("-P"); fputs (HELP_OPTION_DESCRIPTION, stdout); fputs (VERSION_OPTION_DESCRIPTION, stdout); if (chown_mode == CHOWN_CHOWN) diff --git a/src/system.h b/src/system.h index 69aed6ea2..b5d38bb18 100644 --- a/src/system.h +++ b/src/system.h @@ -624,6 +624,24 @@ the VERSION_CONTROL environment variable. Here are the values:\n\ } static inline void +emit_symlink_recurse_options (char const *default_opt) +{ + printf (_("\ +\n\ +The following options modify how a hierarchy is traversed when the -R\n\ +option is also specified. If more than one is specified, only the final\n\ +one takes effect. '%s' is the default.\n\ +\n\ + -H if a command line argument is a symbolic link\n\ + to a directory, traverse it\n\ + -L traverse every symbolic link to a directory\n\ + encountered\n\ + -P do not traverse any symbolic links\n\ +\n\ +"), default_opt); +} + +static inline void emit_exec_status (char const *program) { printf (_("\n\ diff --git a/tests/chmod/symlinks.sh b/tests/chmod/symlinks.sh new file mode 100755 index 000000000..815e8fb5c --- /dev/null +++ b/tests/chmod/symlinks.sh @@ -0,0 +1,87 @@ +#!/bin/sh +# Verify chmod symlink handling options + +# Copyright (C) 2024 Free Software Foundation, Inc. + +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. + +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. + +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src +print_ver_ chmod + +#dirs +mkdir -p a/b a/c || framework_failure_ +#files +touch a/b/file a/c/file || framework_failure_ +#dangling link +ln -s foo a/dangle || framework_failure_ +#link to file +ln -s ../b/file a/c/link || framework_failure_ +#link to dir +ln -s b a/dirlink || framework_failure_ + +# tree -F a +# a/ +# |-- b/ +# | '-- file +# |-- c/ +# | |-- file +# | '-- link -> ../b/file +# |-- dangle -> foo +# '-- dirlink -> b/ + +reset_modes() { chmod 777 a/b a/c a/b/file a/c/file || fail=1; } +count_755() { test "$(grep 'rwxr-xr-x' 'out' | wc -l)" = "$1"; } + +reset_modes +# -R (with default -H) does not deref traversed symlinks (only cli args) +chmod 755 -R a/c || fail=1 +ls -l a/b > out || framework_failure_ +count_755 0 || fail=1 +ls -lR a/c > out || framework_failure_ +count_755 1 || fail=1 + +reset_modes +# set a/c a/c/file and a/b/file (through symlink) to 755 +chmod 755 -LR a/c || fail=1 +ls -ld a/c a/c/file a/b/file > out || framework_failure_ +count_755 3 || { cat out; fail=1; } + +reset_modes +# do not set /a/b/file through symlink (should try to chmod the link itself) +chmod 755 -RP a/c/ || fail=1 +ls -l a/b > out || framework_failure_ +count_755 0 || fail=1 + +reset_modes +# set /a/b/file through symlink +chmod 755 --dereference a/c/link || fail=1 +ls -l a/b > out || framework_failure_ +count_755 1 || fail=1 + +reset_modes +# do not set /a/b/file through symlink (should try to chmod the link itself) +chmod 755 --no-dereference a/c/link 2>err || fail=1 +ls -l a/b > out || framework_failure_ +count_755 0 || fail=1 + +# Dangling links should not induce an error if not dereferencing +for noderef in '-h' '-RP' '-P'; do + chmod 755 --no-dereference a/dangle 2>err || fail=1 +done +# Dangling links should induce an error if dereferencing +for deref in '' '--deref' '-R'; do + returns_ 1 chmod 755 $deref a/dangle 2>err || fail=1 +done + +Exit $fail diff --git a/tests/local.mk b/tests/local.mk index 7cd1ef7b5..4075525ba 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -476,6 +476,7 @@ all_tests = \ tests/chmod/thru-dangling.sh \ tests/chmod/umask-x.sh \ tests/chmod/usage.sh \ + tests/chmod/symlinks.sh \ tests/chown/deref.sh \ tests/chown/preserve-root.sh \ tests/chown/separator.sh \ -- 2.11.4.GIT