From 425b8a2f534fe02e8c1e39ad6a3d2c18eca12de3 Mon Sep 17 00:00:00 2001 From: =?utf8?q?P=C3=A1draig=20Brady?= Date: Tue, 19 Mar 2024 23:34:31 +0000 Subject: [PATCH] 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 --- NEWS | 4 ++++ src/chmod.c | 15 ++++++++------- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/NEWS b/NEWS index 0f4503cbbb..b3004273b6 100644 --- 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 835b75d9ac..b95371d0d3 100644 --- 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) -- 2.47.2