summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLudovic Courtès <ludo@gnu.org>2024-03-11 10:59:42 +0100
committerLudovic Courtès <ludo@gnu.org>2024-03-11 22:12:34 +0100
commit8f4ffb3fae133bb21d7991e97c2f19a7108b1143 (patch)
tree4e626a0c742043b9ce84ebb71e9a1622f76ba557
parenta26bce55e60aa3444c4378d3996f3aa41b9661e9 (diff)
daemon: Protect against FD escape when building fixed-output derivations (CVE-2024-27297).
This fixes a security issue (CVE-2024-27297) whereby a fixed-output derivation build process could open a writable file descriptor to its output, send it to some outside process for instance over an abstract AF_UNIX socket, which would then allow said process to modify the file in the store after it has been marked as “valid”. Vulnerability discovered by puck <https://github.com/puckipedia>. Nix security advisory: https://github.com/NixOS/nix/security/advisories/GHSA-2ffj-w4mj-pg37 Nix fix: https://github.com/NixOS/nix/commit/244f3eee0bbc7f11e9b383a15ed7368e2c4becc9 * nix/libutil/util.cc (readDirectory): Add variants that take a DIR* and a file descriptor. Rewrite the ‘Path’ variant accordingly. (copyFile, copyFileRecursively): New functions. * nix/libutil/util.hh (copyFileRecursively): New declaration. * nix/libstore/build.cc (DerivationGoal::buildDone): When ‘fixedOutput’ is true, call ‘copyFileRecursively’ followed by ‘rename’ on each output. Change-Id: I7952d41093eed26e123e38c14a4c1424be1ce1c4 Reported-by: Picnoir <picnoir@alternativebit.fr>, Théophane Hufschmitt <theophane.hufschmitt@tweag.io> Change-Id: Idb5f2757f35af86b032a9851cecb19b70227bd88
-rw-r--r--nix/libstore/build.cc16
-rw-r--r--nix/libutil/util.cc112
-rw-r--r--nix/libutil/util.hh6
3 files changed, 129 insertions, 5 deletions
diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc
index 461fcbc584..e2adee118b 100644
--- a/nix/libstore/build.cc
+++ b/nix/libstore/build.cc
@@ -1382,6 +1382,22 @@ void DerivationGoal::buildDone()
% drvPath % statusToString(status));
}
+ if (fixedOutput) {
+ /* Replace the output, if it exists, by a fresh copy of itself to
+ make sure that there's no stale file descriptor pointing to it
+ (CVE-2024-27297). */
+ foreach (DerivationOutputs::iterator, i, drv.outputs) {
+ if (pathExists(i->second.path)) {
+ Path pivot = i->second.path + ".tmp";
+ copyFileRecursively(i->second.path, pivot, true);
+ int err = rename(pivot.c_str(), i->second.path.c_str());
+ if (err != 0)
+ throw SysError(format("renaming `%1%' to `%2%'")
+ % pivot % i->second.path);
+ }
+ }
+ }
+
/* Compute the FS closure of the outputs and register them as
being valid. */
registerOutputs();
diff --git a/nix/libutil/util.cc b/nix/libutil/util.cc
index 82eac72120..493f06f357 100644
--- a/nix/libutil/util.cc
+++ b/nix/libutil/util.cc
@@ -215,14 +215,11 @@ bool isLink(const Path & path)
}
-DirEntries readDirectory(const Path & path)
+static DirEntries readDirectory(DIR *dir)
{
DirEntries entries;
entries.reserve(64);
- AutoCloseDir dir = opendir(path.c_str());
- if (!dir) throw SysError(format("opening directory `%1%'") % path);
-
struct dirent * dirent;
while (errno = 0, dirent = readdir(dir)) { /* sic */
checkInterrupt();
@@ -230,11 +227,29 @@ DirEntries readDirectory(const Path & path)
if (name == "." || name == "..") continue;
entries.emplace_back(name, dirent->d_ino, dirent->d_type);
}
- if (errno) throw SysError(format("reading directory `%1%'") % path);
+ if (errno) throw SysError(format("reading directory"));
return entries;
}
+DirEntries readDirectory(const Path & path)
+{
+ AutoCloseDir dir = opendir(path.c_str());
+ if (!dir) throw SysError(format("opening directory `%1%'") % path);
+ return readDirectory(dir);
+}
+
+static DirEntries readDirectory(int fd)
+{
+ /* Since 'closedir' closes the underlying file descriptor, duplicate FD
+ beforehand. */
+ int fdcopy = dup(fd);
+ if (fdcopy < 0) throw SysError("dup");
+
+ AutoCloseDir dir = fdopendir(fdcopy);
+ if (!dir) throw SysError(format("opening directory from file descriptor `%1%'") % fd);
+ return readDirectory(dir);
+}
unsigned char getFileType(const Path & path)
{
@@ -364,6 +379,93 @@ void deletePath(const Path & path, unsigned long long & bytesFreed, size_t linkT
_deletePath(path, bytesFreed, linkThreshold);
}
+static void copyFile(int sourceFd, int destinationFd)
+{
+ struct stat st;
+ if (fstat(sourceFd, &st) == -1) throw SysError("statting file");
+
+ ssize_t result = copy_file_range(sourceFd, NULL, destinationFd, NULL, st.st_size, 0);
+ if (result < 0 && errno == ENOSYS) {
+ for (size_t remaining = st.st_size; remaining > 0; ) {
+ unsigned char buf[8192];
+ size_t count = std::min(remaining, sizeof buf);
+
+ readFull(sourceFd, buf, count);
+ writeFull(destinationFd, buf, count);
+ remaining -= count;
+ }
+ } else {
+ if (result < 0)
+ throw SysError(format("copy_file_range `%1%' to `%2%'") % sourceFd % destinationFd);
+ if (result < st.st_size)
+ throw SysError(format("short write in copy_file_range `%1%' to `%2%'")
+ % sourceFd % destinationFd);
+ }
+}
+
+static void copyFileRecursively(int sourceroot, const Path &source,
+ int destinationroot, const Path &destination,
+ bool deleteSource)
+{
+ struct stat st;
+ if (fstatat(sourceroot, source.c_str(), &st, AT_SYMLINK_NOFOLLOW) == -1)
+ throw SysError(format("statting file `%1%'") % source);
+
+ if (S_ISREG(st.st_mode)) {
+ AutoCloseFD sourceFd = openat(sourceroot, source.c_str(),
+ O_CLOEXEC | O_NOFOLLOW | O_RDONLY);
+ if (sourceFd == -1) throw SysError(format("opening `%1%'") % source);
+
+ AutoCloseFD destinationFd = openat(destinationroot, destination.c_str(),
+ O_CLOEXEC | O_CREAT | O_WRONLY | O_TRUNC,
+ st.st_mode);
+ if (destinationFd == -1) throw SysError(format("opening `%1%'") % source);
+
+ copyFile(sourceFd, destinationFd);
+ } else if (S_ISLNK(st.st_mode)) {
+ char target[st.st_size + 1];
+ ssize_t result = readlinkat(sourceroot, source.c_str(), target, st.st_size);
+ if (result != st.st_size) throw SysError("reading symlink target");
+ target[st.st_size] = '\0';
+ int err = symlinkat(target, destinationroot, destination.c_str());
+ if (err != 0)
+ throw SysError(format("creating symlink `%1%'") % destination);
+ } else if (S_ISDIR(st.st_mode)) {
+ int err = mkdirat(destinationroot, destination.c_str(), 0755);
+ if (err != 0)
+ throw SysError(format("creating directory `%1%'") % destination);
+
+ AutoCloseFD destinationFd = openat(destinationroot, destination.c_str(),
+ O_CLOEXEC | O_RDONLY | O_DIRECTORY);
+ if (err != 0)
+ throw SysError(format("opening directory `%1%'") % destination);
+
+ AutoCloseFD sourceFd = openat(sourceroot, source.c_str(),
+ O_CLOEXEC | O_NOFOLLOW | O_RDONLY);
+ if (sourceFd == -1)
+ throw SysError(format("opening `%1%'") % source);
+
+ if (deleteSource && !(st.st_mode & S_IWUSR)) {
+ /* Ensure the directory 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);
+ }
+
+ for (auto & i : readDirectory(sourceFd))
+ copyFileRecursively((int)sourceFd, i.name, (int)destinationFd, i.name,
+ deleteSource);
+ } else throw Error(format("refusing to copy irregular file `%1%'") % source);
+
+ if (deleteSource)
+ unlinkat(sourceroot, source.c_str(),
+ S_ISDIR(st.st_mode) ? AT_REMOVEDIR : 0);
+}
+
+void copyFileRecursively(const Path &source, const Path &destination, bool deleteSource)
+{
+ copyFileRecursively(AT_FDCWD, source, AT_FDCWD, destination, deleteSource);
+}
static Path tempName(Path tmpRoot, const Path & prefix, bool includePid,
int & counter)
diff --git a/nix/libutil/util.hh b/nix/libutil/util.hh
index 880b0e93b2..058f5f8446 100644
--- a/nix/libutil/util.hh
+++ b/nix/libutil/util.hh
@@ -102,6 +102,12 @@ void deletePath(const Path & path);
void deletePath(const Path & path, unsigned long long & bytesFreed,
size_t linkThreshold = 1);
+/* Copy SOURCE to DESTINATION, recursively. Throw if SOURCE contains a file
+ that is not a regular file, symlink, or directory. When DELETESOURCE is
+ true, delete source files once they have been copied. */
+void copyFileRecursively(const Path &source, const Path &destination,
+ bool deleteSource = false);
+
/* Create a temporary directory. */
Path createTempDir(const Path & tmpRoot = "", const Path & prefix = "nix",
bool includePid = true, bool useGlobalCounter = true, mode_t mode = 0755);