From 0e79d5b6550729e6ce3bac1e979638ac054ba5a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= Date: Tue, 15 Apr 2025 14:46:45 +0200 Subject: [PATCH] =?UTF-8?q?daemon:=20Protect=20=E2=80=98copyFileRecursivel?= =?UTF-8?q?y=E2=80=99=20from=20race=20conditions.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, if an attacker managed to introduce a hard link or a symlink on one of the destination file names before it is opened, ‘copyFileRecursively’ would overwrite the symlink’s target or the hard link’s content. This kind of attack could be carried out while guix-daemon is copying the output or the chroot directory of a failed fixed-output derivation build, possibly allowing the attacker to escalate to the privileges of the build user. * nix/libutil/util.cc (copyFileRecursively): In the ‘S_ISREG’ case, open ‘destination’ with O_NOFOLLOW | O_EXCL. In the ‘S_ISDIR’ case, open ‘destination’ with O_NOFOLLOW. Reported-by: Reepca Russelstein Change-Id: I94273efe4e92c1a4270a98c5ec47bd098e9227c9 Signed-off-by: John Kehayias --- nix/libutil/util.cc | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/nix/libutil/util.cc b/nix/libutil/util.cc index 327edf471f0..8938a213f6d 100644 --- a/nix/libutil/util.cc +++ b/nix/libutil/util.cc @@ -473,7 +473,8 @@ static void copyFileRecursively(int sourceroot, const Path &source, if (sourceFd == -1) throw SysError(format("opening `%1%'") % source); AutoCloseFD destinationFd = openat(destinationroot, destination.c_str(), - O_CLOEXEC | O_CREAT | O_WRONLY | O_TRUNC, + O_CLOEXEC | O_CREAT | O_WRONLY | O_TRUNC + | O_NOFOLLOW | O_EXCL, st.st_mode); if (destinationFd == -1) throw SysError(format("opening `%1%'") % source); @@ -495,7 +496,8 @@ static void copyFileRecursively(int sourceroot, const Path &source, throw SysError(format("creating directory `%1%'") % destination); AutoCloseFD destinationFd = openat(destinationroot, destination.c_str(), - O_CLOEXEC | O_RDONLY | O_DIRECTORY); + O_CLOEXEC | O_RDONLY | O_DIRECTORY + | O_NOFOLLOW); if (err != 0) throw SysError(format("opening directory `%1%'") % destination); @@ -505,7 +507,7 @@ static void copyFileRecursively(int sourceroot, const Path &source, throw SysError(format("opening `%1%'") % source); if (deleteSource && !(st.st_mode & S_IWUSR)) { - /* Ensure the directory writable so files within it can be + /* Ensure the directory is writable so files within it can be deleted. */ if (fchmod(sourceFd, st.st_mode | S_IWUSR) == -1) throw SysError(format("making `%1%' directory writable") % source);