bug#10311: RFE: Give chmod a "-h" option as well

[Paul Eggert]

On 12/16/11 09:30, Bob Proulx wrote:
> Neither chmod -R nor find by default dereference symlinks.

But there's still a problem without -R.  Suppose the attacker does
"ln -s /etc/passwd slylink" and then root does "chmod a+w *"
in a directory containing slylink.  Then anyone can write /etc/passwd.

There's an obvious fix here: make chmod act like chown with respect
to the -H, -L, -P, and -h options.  This would be compatible with
FreeBSD chmod.  On hosts that support symbolic-link permissions
(BSD being one of them), GNU chmod would act like BSD chmod when
changing the mode of symbolic links.  On hosts that don't, it
would report an error.  Then a paranoid root (and shouldn't root
be paranoid? :-) could use chmod -h for everything.

Here's a quick and untested patch to do this.  Assuming there's interest,
I can flesh this out by adding proper documentation and test cases.

diff --git a/src/chmod.c b/src/chmod.c
index 6fec84a..6b7c021 100644
--- a/src/chmod.c
+++ b/src/chmod.c
@@ -68,6 +68,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;
 
@@ -87,7 +91,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
 };
@@ -95,7 +100,9 @@ enum
 static struct option const long_options[] =
 {
   {"changes", no_argument, NULL, 'c'},
+  {"dereference", no_argument, NULL, DEREFERENCE_OPTION},
   {"recursive", no_argument, NULL, 'R'},
+  {"no-dereference", no_argument, NULL, 'h'},
   {"no-preserve-root", no_argument, NULL, NO_PRESERVE_ROOT},
   {"preserve-root", no_argument, NULL, PRESERVE_ROOT},
   {"quiet", no_argument, NULL, 'f'},
@@ -188,6 +195,7 @@ process_file (FTS *fts, FTSENT *ent)
   char const *file_full_name = ent->fts_path;
   char const *file = ent->fts_accpath;
   const struct stat *file_stats = ent->fts_statp;
+  struct stat stat_buf;
   mode_t old_mode IF_LINT ( = 0);
   mode_t new_mode IF_LINT ( = 0);
   bool ok = true;
@@ -232,10 +240,28 @@ process_file (FTS *fts, FTSENT *ent)
       break;
 
     case FTS_SLNONE:
-      if (! force_silent)
-        error (0, 0, _("cannot operate on dangling symlink %s"),
-               quote (file_full_name));
-      ok = false;
+      if (dereference)
+        {
+          if (! force_silent)
+            error (0, 0, _("cannot operate on dangling symlink %s"),
+                   quote (file_full_name));
+          ok = false;
+        }
+      break;
+
+    case FTS_SL:
+      if (dereference)
+        {
+          if (fstatat (fts->fts_cwd_fd, file, &stat_buf, 0) != 0)
+            {
+              if (! force_silent)
+                error (0, errno, _("cannot dereference %s"),
+                       quote (file_full_name));
+              ok = false;
+            }
+
+          file_stats = &stat_buf;
+        }
       break;
 
     case FTS_DC:               /* directory that causes cycles */
@@ -266,17 +292,16 @@ process_file (FTS *fts, FTSENT *ent)
       new_mode = mode_adjust (old_mode, S_ISDIR (old_mode) != 0, umask_value,
                               change, NULL);
 
-      if (! S_ISLNK (old_mode))
+      if (fchmodat (fts->fts_cwd_fd, file, new_mode,
+                    dereference ? 0 : AT_SYMLINK_NOFOLLOW)
+          == 0)
+        chmod_succeeded = true;
+      else
         {
-          if (chmodat (fts->fts_cwd_fd, file, new_mode) == 0)
-            chmod_succeeded = true;
-          else
-            {
-              if (! force_silent)
-                error (0, errno, _("changing permissions of %s"),
-                       quote (file_full_name));
-              ok = false;
-            }
+          if (! force_silent)
+            error (0, errno, _("changing permissions of %s"),
+                   quote (file_full_name));
+          ok = false;
         }
     }
 
@@ -381,6 +406,13 @@ Change the mode of each FILE to MODE.\n\
   -c, --changes           like verbose but report only when a change is made\n\
 "), stdout);
       fputs (_("\
+      --dereference      affect the referent of each symbolic link (this is\n\
+                         the default), rather than the symbolic link itself\n\
+  -h, --no-dereference   affect each symbolic link instead of any referenced\n\
+                         file (useful only on systems that can change the\n\
+                         ownership of a symlink)\n\
+"), stdout);
+      fputs (_("\
       --no-preserve-root  do not treat `/' specially (the default)\n\
       --preserve-root     fail to operate recursively on `/'\n\
 "), stdout);
@@ -389,6 +421,19 @@ Change the mode of each FILE to MODE.\n\
   -v, --verbose           output a diagnostic for every file processed\n\
       --reference=RFILE   use RFILE's mode instead of MODE values\n\
   -R, --recursive         change files and directories recursively\n\
+\n\
+"), stdout);
+      fputs (_("\
+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);
       fputs (HELP_OPTION_DESCRIPTION, stdout);
       fputs (VERSION_OPTION_DESCRIPTION, stdout);
@@ -414,6 +459,7 @@ main (int argc, char **argv)
   bool preserve_root = false;
   char const *reference_file = NULL;
   int c;
+  int bit_flags = FTS_PHYSICAL;
 
   initialize_main (&argc, &argv);
   set_program_name (argv[0]);
@@ -426,12 +472,33 @@ 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::,::+::=::",
                            long_options, NULL))
          != -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':
@@ -499,6 +566,21 @@ 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;
+        }
+    }
+  else
+    {
+      bit_flags = FTS_PHYSICAL;
+    }
+
   if (reference_file)
     {
       if (mode)
@@ -553,8 +635,8 @@ main (int argc, char **argv)
       root_dev_ino = NULL;
     }
 
-  ok = process_files (argv + optind,
-                      FTS_COMFOLLOW | FTS_PHYSICAL | FTS_DEFER_STAT);
+  bit_flags |= FTS_DEFER_STAT;
+  ok = process_files (argv + optind, bit_flags);
 
   exit (ok ? EXIT_SUCCESS : EXIT_FAILURE);
 }