chmod: fix TOCTOU security issue with symlink replacement

This is an issue with -[H]R mode, where an attacker
may replace a traversed file with a symlink
between where we stat() the file and chmod() the file.

* src/chmod.c (process_file): Remove the first !S_ISLNK guard
as that's now just an optimization, and instead consistently
apply fchmodat() to files/symlinks.  Ensure AT_SYMLINK_NOFOLLOW
is set when traversing in default (-H) mode.
* NEWS: Mention the bug fix.
Fixes https://bugs.gnu.org/11108

diff --git a/NEWS b/NEWS
index 0f4503cbbb37be7ca359d33b748ad61e3c83f8ff..b3004273b6aaae8b3021e7bdfbfae17f8f77c281 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,10 @@ GNU coreutils NEWS                                    -*- outline -*-
 
 ** Bug fixes
 
+  chmod -R now avoids a race where an attacker may replace a traversed file
+  with a symlink, causing chmod to operate on an unintended file.
+  [This bug was present in "the beginning".]
+
   cp, mv, and install no longer issue spurious diagnostics like "failed
   to preserve ownership" when copying to GNU/Linux CIFS file systems.
   They do this by working around some Linux CIFS bugs.

diff --git a/src/chmod.c b/src/chmod.c
index 835b75d9acfccd9dc62d0ce9134684c9e4c3c9da..b95371d0d30f36b9c1c99c280bc7dddf8b425fa2 100644 (file)
--- a/src/chmod.c
+++ b/src/chmod.c
@@ -301,18 +301,16 @@ process_file (FTS *fts, FTSENT *ent)
       return false;
     }
 
-  /* 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))
+  if (ch.status == CH_NOT_APPLIED)
     {
       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);
-      /* XXX: Racy if FILE is now replaced with a symlink, which is
-         a potential security issue with -[H]R.  */
+      bool follow_symlink = !!dereference;
+      if (dereference == -1) /* -H with/without -R, -P without -R.  */
+        follow_symlink = ent->fts_level == 0;
       if (fchmodat (fts->fts_cwd_fd, file, ch.new_mode,
-                    dereference ? 0 : AT_SYMLINK_NOFOLLOW) == 0)
+                    follow_symlink ? 0 : AT_SYMLINK_NOFOLLOW) == 0)
         ch.status = CH_SUCCEEDED;
       else
         {
@@ -590,6 +588,9 @@ main (int argc, char **argv)
         }
     }
 
+  if (dereference == -1 && bit_flags == FTS_LOGICAL)
+    dereference = 1;
+
   if (reference_file)
     {
       if (mode)