Fix TOCTOU race in daemon status file handling. (#19924)
On most UNIX systems, including Linux, macOS, and FreeBSD, the `chmod()` function will follow symlinks if called on a symlink. This means that any code that uses `chmod()` to apply permissions to a known filename can be exploited to apply those same permissions to an arbitrary file on the system that the user the code is running as could call `chmod()` on by simply replacing the known file with a symlink pointing to the target file after the known file is created but before its permissions are modified.: Because we are already opening the file in question in this case, we should instead use `fchmod()` to modify the permissions via the file descriptor before closing the file, which is not vulnerable to this race condition.
Austin S. Hemmelgarn committed
Mar 20, 2025 at 15:04 UTC
3321195f7f6a916a5738fcbbe684782f0ece97d9
1 file changed
+8
-7
src/daemon/daemon-status-file.c
+8
-7
@@ -747,11 +747,11 @@ static bool save_status_file(const char *directory, const char *content, size_t
747
// THIS FUNCTION MUST USE ONLY ASYNC-SIGNAL-SAFE OPERATIONS
748
749
// Linux: https://man7.org/linux/man-pages/man7/signal-safety.7.html
750
- // memcpy(), strlen(), open(), write(), fsync(), close(), chmod(), rename(), unlink()
750
+ // memcpy(), strlen(), open(), write(), fsync(), close(), fchmod(), rename(), unlink()
751
752
// MacOS: https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/sigaction.2.html#//apple_ref/doc/man/2/sigaction
753
- // open(), write(), fsync(), close(), chmod(), rename(), unlink()
754
- // does not explicitly mention memcpy() and strlen(), but they are safe
753
+ // open(), write(), fsync(), close(), rename(), unlink()
754
+ // does not explicitly mention fchmod, memcpy(), and strlen(), but they are safe
755
756
if(!directory || !*directory)
757
return false;
@@ -813,14 +813,15 @@ static bool save_status_file(const char *directory, const char *content, size_t
813
return false;
814
}
815
816
- /* Close file */
817
- if (close(fd) == -1) {
816
+ /* Set permissions using chmod() */
817
+ if (fchmod(fd, 0664) != 0) {
818
+ close(fd);
819
unlink(temp_filename);
820
return false;
821
}
822
822
- /* Set permissions using chmod() */
823
- if (chmod(temp_filename, 0664) != 0) {
823
+ /* Close file */
824
+ if (close(fd) == -1) {
825
unlink(temp_filename);
826
return false;
827
}