diff options
| author | Pádraig Brady <P@draigBrady.com> | 2024-03-19 23:34:31 +0000 |
|---|---|---|
| committer | Pádraig Brady <P@draigBrady.com> | 2024-03-19 23:56:45 +0000 |
| commit | 425b8a2f534fe02e8c1e39ad6a3d2c18eca12de3 (patch) | |
| tree | ecd1d754d19dce0c8d3e6bf0e89607b211ee6af2 | |
| parent | chmod: add support for -h, -H,-L,-P, --dereference options (diff) | |
| download | coreutils-425b8a2f534fe02e8c1e39ad6a3d2c18eca12de3.tar.gz coreutils-425b8a2f534fe02e8c1e39ad6a3d2c18eca12de3.zip | |
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
| -rw-r--r-- | NEWS | 4 | ||||
| -rw-r--r-- | src/chmod.c | 15 |
2 files changed, 12 insertions, 7 deletions
@@ -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 835b75d9a..b95371d0d 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) |