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

@@ -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.
 

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

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

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.  */
         }
     }
 
@@ -387,6 +427,11 @@ With --reference, change the mode of each FILE to that of RFILE.\n\
   -c, --changes          like verbose but report only when a change is made\n\
   -f, --silent, --quiet  suppress most error messages\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\
@@ -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);
 }

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)

src/system.h

@@ -623,6 +623,24 @@ the VERSION_CONTROL environment variable.  Here are the values:\n\
 "), stdout);
 }
 
+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)
 {

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 <http://www.gnu.org/licenses/>.
+
+. "${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

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			\