diff --git a/pkgs/tools/package-management/nix/default.nix b/pkgs/tools/package-management/nix/default.nix index 7d3c6fbfcc59..449da9289815 100644 --- a/pkgs/tools/package-management/nix/default.nix +++ b/pkgs/tools/package-management/nix/default.nix @@ -171,34 +171,39 @@ lib.makeExtensible ( nix_2_24 = commonAutoconf { version = "2.24.14"; hash = "sha256-SthMCsj6POjawLnJq9+lj/UzObX9skaeN1UGmMZiwTY="; + patches = [ ./patches/ghsa-g948-229j-48j3-2.24.patch ]; self_attribute_name = "nix_2_24"; }; nix_2_26 = commonMeson { version = "2.26.3"; hash = "sha256-5ZV8YqU8mfFmoAMiUEuBqNwk0T3vUR//x1D12BiYCeY="; + patches = [ ./patches/ghsa-g948-229j-48j3-2.26.patch ]; self_attribute_name = "nix_2_26"; }; nix_2_28 = commonMeson { version = "2.28.3"; hash = "sha256-TjZp5ITSUvNRAzNznmkZRQxNRzMLiSAplz4bV2T8cbs="; + patches = [ ./patches/ghsa-g948-229j-48j3-2.28.patch ]; self_attribute_name = "nix_2_28"; }; - nixComponents_2_29 = nixDependencies.callPackage ./modular/packages.nix rec { - version = "2.29.0"; - inherit (self.nix_2_24.meta) maintainers teams; - otherSplices = generateSplicesForNixComponents "nixComponents_2_29"; - src = fetchFromGitHub { - # FIXME: back to NixOS org once they fix it - owner = "vcunat"; - repo = "nix"; - rev = "p/jq-1.8.0"; # just a tiny test-only patch atop 2.29.0 - # see https://github.com/NixOS/nix/pull/13371 - hash = "sha256-F2ZODsET4cBsgsyOi8Sg/quESU0DnrYri0hYniqu37k="; - }; - }; + nixComponents_2_29 = + (nixDependencies.callPackage ./modular/packages.nix rec { + version = "2.29.0"; + inherit (self.nix_2_24.meta) maintainers teams; + otherSplices = generateSplicesForNixComponents "nixComponents_2_29"; + src = fetchFromGitHub { + # FIXME: back to NixOS org once they fix it + owner = "vcunat"; + repo = "nix"; + rev = "p/jq-1.8.0"; # just a tiny test-only patch atop 2.29.0 + # see https://github.com/NixOS/nix/pull/13371 + hash = "sha256-F2ZODsET4cBsgsyOi8Sg/quESU0DnrYri0hYniqu37k="; + }; + }).appendPatches + [ ./patches/ghsa-g948-229j-48j3-2.29.patch ]; nix_2_29 = addTests "nix_2_29" self.nixComponents_2_29.nix-everything; diff --git a/pkgs/tools/package-management/nix/patches/ghsa-g948-229j-48j3-2.24.patch b/pkgs/tools/package-management/nix/patches/ghsa-g948-229j-48j3-2.24.patch new file mode 100644 index 000000000000..27b16dc6b8c6 --- /dev/null +++ b/pkgs/tools/package-management/nix/patches/ghsa-g948-229j-48j3-2.24.patch @@ -0,0 +1,436 @@ +From b0fab9f90b397a2b02f41df5f467ae3cf8b91c3c Mon Sep 17 00:00:00 2001 +From: Eelco Dolstra +Date: Thu, 19 Jun 2025 16:20:34 +0200 +Subject: [PATCH] Fixes for GHSA-g948-229j-48j3 +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Squashed commit of the following: + +commit 04fff3a637d455cbb1d75937a235950e43008db9 +Author: Eelco Dolstra +Date: Thu Jun 12 12:30:32 2025 +0200 + + Chown structured attr files safely + +commit 5417ad445e414c649d0cfc71a05661c7bf8f3ef5 +Author: Eelco Dolstra +Date: Thu Jun 12 12:14:04 2025 +0200 + + Replace 'bool sync' with an enum for clarity + + And drop writeFileAndSync(). + +commit 7ae0141f328d8e8e1094be24665789c05f974ba6 +Author: Eelco Dolstra +Date: Thu Jun 12 11:35:28 2025 +0200 + + Drop guessOrInventPathFromFD() + + No need to do hacky stuff like that when we already know the original path. + +commit 45b05098bd019da7c57cd4227a89bfd0fa65bb08 +Author: Eelco Dolstra +Date: Thu Jun 12 11:15:58 2025 +0200 + + Tweak comment + +commit 0af15b31209d1b7ec8addfae9a1a6b60d8f35848 +Author: Raito Bezarius +Date: Thu Mar 27 12:22:26 2025 +0100 + + libstore: ensure that temporary directory is always 0o000 before deletion + + In the case the deletion fails, we should ensure that the temporary + directory cannot be used for nefarious purposes. + + Change-Id: I498a2dd0999a74195d13642f44a5de1e69d46120 + Signed-off-by: Raito Bezarius + +commit 2c20fa37b15cfa03ac6a1a6a47cdb2ed66c0827e +Author: Raito Bezarius +Date: Wed Mar 26 12:42:55 2025 +0100 + + libutil: ensure that `_deletePath` does NOT use absolute paths with dirfds + + When calling `_deletePath` with a parent file descriptor, `openat` is + made effective by using relative paths to the directory file descriptor. + + To avoid the problem, the signature is changed to resist misuse with an + assert in the prologue of the function. + + Change-Id: I6b3fc766bad2afe54dc27d47d1df3873e188de96 + Signed-off-by: Raito Bezarius + +commit d3c370bbcae48bb825ce19fd0f73bb4eefd2c9ea +Author: Raito Bezarius +Date: Wed Mar 26 01:07:47 2025 +0100 + + libstore: ensure that `passAsFile` is created in the original temp dir + + This ensures that `passAsFile` data is created inside the expected + temporary build directory by `openat()` from the parent directory file + descriptor. + + This avoids a TOCTOU which is part of the attack chain of CVE-????. + + Change-Id: Ie5273446c4a19403088d0389ae8e3f473af8879a + Signed-off-by: Raito Bezarius + +commit 45d3598724f932d024ef6bc2ffb00c1bb90e6018 +Author: Raito Bezarius +Date: Wed Mar 26 01:06:03 2025 +0100 + + libutil: writeFile variant for file descriptors + + `writeFile` lose its `sync` boolean flag to make things simpler. + + A new `writeFileAndSync` function is created and all call sites are + converted to it. + + Change-Id: Ib871a5283a9c047db1e4fe48a241506e4aab9192 + Signed-off-by: Raito Bezarius + +commit 732bd9b98cabf4aaf95a01fd318923de303f9996 +Author: Raito Bezarius +Date: Wed Mar 26 01:05:34 2025 +0100 + + libstore: chown to builder variant for file descriptors + + We use it immediately for the build temporary directory. + + Change-Id: I180193c63a2b98721f5fb8e542c4e39c099bb947 + Signed-off-by: Raito Bezarius + +commit 962c65f8dcd5570dd92c72370a862c7b38942e0d +Author: Raito Bezarius +Date: Wed Mar 26 01:04:59 2025 +0100 + + libstore: open build directory as a dirfd as well + + We now keep around a proper AutoCloseFD around the temporary directory + which we plan to use for openat operations and avoiding the build + directory being swapped out while we are doing something else. + + Change-Id: I18d387b0f123ebf2d20c6405cd47ebadc5505f2a + Signed-off-by: Raito Bezarius + +commit c9b42462b75b5a37ee6564c2b53cff186c8323da +Author: Raito Bezarius +Date: Wed Mar 26 01:04:12 2025 +0100 + + libutil: guess or invent a path from file descriptors + + This is useful for certain error recovery paths (no pun intended) that + does not thread through the original path name. + + Change-Id: I2d800740cb4f9912e64c923120d3f977c58ccb7e + Signed-off-by: Raito Bezarius + +Signed-off-by: Jörg Thalheim +--- + src/libstore/local-store.cc | 6 +-- + .../unix/build/local-derivation-goal.cc | 46 ++++++++++++++---- + .../unix/build/local-derivation-goal.hh | 20 ++++++++ + src/libutil/file-system.cc | 47 +++++++++++-------- + src/libutil/file-system.hh | 8 +++- + 5 files changed, 94 insertions(+), 33 deletions(-) + +diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc +index c6e3af456..c5489444e 100644 +--- a/src/libstore/local-store.cc ++++ b/src/libstore/local-store.cc +@@ -187,7 +187,7 @@ void migrateCASchema(SQLite& db, Path schemaPath, AutoCloseFD& lockFd) + txn.commit(); + } + +- writeFile(schemaPath, fmt("%d", nixCASchemaVersion), 0666, true); ++ writeFile(schemaPath, fmt("%d", nixCASchemaVersion), 0666, FsSync::Yes); + lockFile(lockFd.get(), ltRead, true); + } + } +@@ -345,7 +345,7 @@ LocalStore::LocalStore( + else if (curSchema == 0) { /* new store */ + curSchema = nixSchemaVersion; + openDB(*state, true); +- writeFile(schemaPath, fmt("%1%", curSchema), 0666, true); ++ writeFile(schemaPath, fmt("%1%", curSchema), 0666, FsSync::Yes); + } + + else if (curSchema < nixSchemaVersion) { +@@ -394,7 +394,7 @@ LocalStore::LocalStore( + txn.commit(); + } + +- writeFile(schemaPath, fmt("%1%", nixSchemaVersion), 0666, true); ++ writeFile(schemaPath, fmt("%1%", nixSchemaVersion), 0666, FsSync::Yes); + + lockFile(globalLock.get(), ltRead, true); + } +diff --git a/src/libstore/unix/build/local-derivation-goal.cc b/src/libstore/unix/build/local-derivation-goal.cc +index f8824e9ce..82c79f361 100644 +--- a/src/libstore/unix/build/local-derivation-goal.cc ++++ b/src/libstore/unix/build/local-derivation-goal.cc +@@ -526,7 +526,14 @@ void LocalDerivationGoal::startBuilder() + } else { + tmpDir = topTmpDir; + } +- chownToBuilder(tmpDir); ++ ++ /* The TOCTOU between the previous mkdir call and this open call is unavoidable due to ++ POSIX semantics.*/ ++ tmpDirFd = AutoCloseFD{open(tmpDir.c_str(), O_RDONLY | O_NOFOLLOW | O_DIRECTORY)}; ++ if (!tmpDirFd) ++ throw SysError("failed to open the build temporary directory descriptor '%1%'", tmpDir); ++ ++ chownToBuilder(tmpDirFd.get(), tmpDir); + + for (auto & [outputName, status] : initialOutputs) { + /* Set scratch path we'll actually use during the build. +@@ -1110,9 +1117,7 @@ void LocalDerivationGoal::initTmpDir() { + } else { + auto hash = hashString(HashAlgorithm::SHA256, i.first); + std::string fn = ".attr-" + hash.to_string(HashFormat::Nix32, false); +- Path p = tmpDir + "/" + fn; +- writeFile(p, rewriteStrings(i.second, inputRewrites)); +- chownToBuilder(p); ++ writeBuilderFile(fn, rewriteStrings(i.second, inputRewrites)); + env[i.first + "Path"] = tmpDirInSandbox + "/" + fn; + } + } +@@ -1217,11 +1222,9 @@ void LocalDerivationGoal::writeStructuredAttrs() + + auto jsonSh = writeStructuredAttrsShell(json); + +- writeFile(tmpDir + "/.attrs.sh", rewriteStrings(jsonSh, inputRewrites)); +- chownToBuilder(tmpDir + "/.attrs.sh"); ++ writeBuilderFile(".attrs.sh", rewriteStrings(jsonSh, inputRewrites)); + env["NIX_ATTRS_SH_FILE"] = tmpDirInSandbox + "/.attrs.sh"; +- writeFile(tmpDir + "/.attrs.json", rewriteStrings(json.dump(), inputRewrites)); +- chownToBuilder(tmpDir + "/.attrs.json"); ++ writeBuilderFile(".attrs.json", rewriteStrings(json.dump(), inputRewrites)); + env["NIX_ATTRS_JSON_FILE"] = tmpDirInSandbox + "/.attrs.json"; + } + } +@@ -1730,6 +1733,24 @@ void setupSeccomp() + #endif + } + ++void LocalDerivationGoal::chownToBuilder(int fd, const Path & path) ++{ ++ if (!buildUser) return; ++ if (fchown(fd, buildUser->getUID(), buildUser->getGID()) == -1) ++ throw SysError("cannot change ownership of file '%1%'", path); ++} ++ ++void LocalDerivationGoal::writeBuilderFile( ++ const std::string & name, ++ std::string_view contents) ++{ ++ auto path = std::filesystem::path(tmpDir) / name; ++ AutoCloseFD fd{openat(tmpDirFd.get(), name.c_str(), O_WRONLY | O_TRUNC | O_CREAT | O_CLOEXEC | O_EXCL | O_NOFOLLOW, 0666)}; ++ if (!fd) ++ throw SysError("creating file %s", path); ++ writeFile(fd, path, contents); ++ chownToBuilder(fd.get(), path); ++} + + void LocalDerivationGoal::runChild() + { +@@ -3006,6 +3027,15 @@ void LocalDerivationGoal::checkOutputs(const std::mapisBuiltin()) { +diff --git a/src/libstore/unix/build/local-derivation-goal.hh b/src/libstore/unix/build/local-derivation-goal.hh +index bf25cf2a6..69c517c4a 100644 +--- a/src/libstore/unix/build/local-derivation-goal.hh ++++ b/src/libstore/unix/build/local-derivation-goal.hh +@@ -37,6 +37,11 @@ struct LocalDerivationGoal : public DerivationGoal + */ + Path topTmpDir; + ++ /** ++ * The file descriptor of the temporary directory. ++ */ ++ AutoCloseFD tmpDirFd; ++ + /** + * The path of the temporary directory in the sandbox. + */ +@@ -232,9 +237,24 @@ struct LocalDerivationGoal : public DerivationGoal + + /** + * Make a file owned by the builder. ++ * ++ * SAFETY: this function is prone to TOCTOU as it receives a path and not a descriptor. ++ * It's only safe to call in a child of a directory only visible to the owner. + */ + void chownToBuilder(const Path & path); + ++ /** ++ * Make a file owned by the builder addressed by its file descriptor. ++ */ ++ void chownToBuilder(int fd, const Path & path); ++ ++ /** ++ * Create a file in `tmpDir` owned by the builder. ++ */ ++ void writeBuilderFile( ++ const std::string & name, ++ std::string_view contents); ++ + int getChildStatus() override; + + /** +diff --git a/src/libutil/file-system.cc b/src/libutil/file-system.cc +index 8ec38e73b..554214d66 100644 +--- a/src/libutil/file-system.cc ++++ b/src/libutil/file-system.cc +@@ -247,7 +247,7 @@ void readFile(const Path & path, Sink & sink) + } + + +-void writeFile(const Path & path, std::string_view s, mode_t mode, bool sync) ++void writeFile(const Path & path, std::string_view s, mode_t mode, FsSync sync) + { + AutoCloseFD fd = toDescriptor(open(path.c_str(), O_WRONLY | O_TRUNC | O_CREAT + // TODO +@@ -257,22 +257,29 @@ void writeFile(const Path & path, std::string_view s, mode_t mode, bool sync) + , mode)); + if (!fd) + throw SysError("opening file '%1%'", path); ++ ++ writeFile(fd, path, s, mode, sync); ++ ++ /* Close explicitly to propagate the exceptions. */ ++ fd.close(); ++} ++ ++void writeFile(AutoCloseFD & fd, const Path & origPath, std::string_view s, mode_t mode, FsSync sync) ++{ ++ assert(fd); + try { + writeFull(fd.get(), s); ++ ++ if (sync == FsSync::Yes) ++ fd.fsync(); ++ + } catch (Error & e) { +- e.addTrace({}, "writing file '%1%'", path); ++ e.addTrace({}, "writing file '%1%'", origPath); + throw; + } +- if (sync) +- fd.fsync(); +- // Explicitly close to make sure exceptions are propagated. +- fd.close(); +- if (sync) +- syncParent(path); + } + +- +-void writeFile(const Path & path, Source & source, mode_t mode, bool sync) ++void writeFile(const Path & path, Source & source, mode_t mode, FsSync sync) + { + AutoCloseFD fd = toDescriptor(open(path.c_str(), O_WRONLY | O_TRUNC | O_CREAT + // TODO +@@ -296,11 +303,11 @@ void writeFile(const Path & path, Source & source, mode_t mode, bool sync) + e.addTrace({}, "writing file '%1%'", path); + throw; + } +- if (sync) ++ if (sync == FsSync::Yes) + fd.fsync(); + // Explicitly close to make sure exceptions are propagated. + fd.close(); +- if (sync) ++ if (sync == FsSync::Yes) + syncParent(path); + } + +@@ -318,7 +325,8 @@ static void _deletePath(Descriptor parentfd, const fs::path & path, uint64_t & b + #ifndef _WIN32 + checkInterrupt(); + +- std::string name(baseNameOf(path.native())); ++ std::string name(path.filename()); ++ assert(name != "." && name != ".." && !name.empty()); + + struct stat st; + if (fstatat(parentfd, name.c_str(), &st, +@@ -359,7 +367,7 @@ static void _deletePath(Descriptor parentfd, const fs::path & path, uint64_t & b + throw SysError("chmod '%1%'", path); + } + +- int fd = openat(parentfd, path.c_str(), O_RDONLY); ++ int fd = openat(parentfd, name.c_str(), O_RDONLY | O_DIRECTORY | O_NOFOLLOW); + if (fd == -1) + throw SysError("opening directory '%1%'", path); + AutoCloseDir dir(fdopendir(fd)); +@@ -371,7 +379,7 @@ static void _deletePath(Descriptor parentfd, const fs::path & path, uint64_t & b + checkInterrupt(); + std::string childName = dirent->d_name; + if (childName == "." || childName == "..") continue; +- _deletePath(dirfd(dir.get()), path + "/" + childName, bytesFreed); ++ _deletePath(dirfd(dir.get()), path / childName, bytesFreed); + } + if (errno) throw SysError("reading directory '%1%'", path); + } +@@ -389,14 +397,13 @@ static void _deletePath(Descriptor parentfd, const fs::path & path, uint64_t & b + + static void _deletePath(const fs::path & path, uint64_t & bytesFreed) + { +- Path dir = dirOf(path.string()); +- if (dir == "") +- dir = "/"; ++ assert(path.is_absolute()); ++ assert(path.parent_path() != path); + +- AutoCloseFD dirfd = toDescriptor(open(dir.c_str(), O_RDONLY)); ++ AutoCloseFD dirfd = toDescriptor(open(path.parent_path().string().c_str(), O_RDONLY)); + if (!dirfd) { + if (errno == ENOENT) return; +- throw SysError("opening directory '%1%'", path); ++ throw SysError("opening directory %s", path.parent_path()); + } + + _deletePath(dirfd.get(), path, bytesFreed); +diff --git a/src/libutil/file-system.hh b/src/libutil/file-system.hh +index ed1112c7e..32b84456d 100644 +--- a/src/libutil/file-system.hh ++++ b/src/libutil/file-system.hh +@@ -148,12 +148,16 @@ Descriptor openDirectory(const std::filesystem::path & path); + std::string readFile(const Path & path); + void readFile(const Path & path, Sink & sink); + ++enum struct FsSync { Yes, No }; ++ + /** + * Write a string to a file. + */ +-void writeFile(const Path & path, std::string_view s, mode_t mode = 0666, bool sync = false); ++void writeFile(const Path & path, std::string_view s, mode_t mode = 0666, FsSync sync = FsSync::No); ++ ++void writeFile(const Path & path, Source & source, mode_t mode = 0666, FsSync sync = FsSync::No); + +-void writeFile(const Path & path, Source & source, mode_t mode = 0666, bool sync = false); ++void writeFile(AutoCloseFD & fd, const Path & origPath, std::string_view s, mode_t mode = 0666, FsSync sync = FsSync::No); + + /** + * Flush a file's parent directory to disk +-- +2.44.1 + diff --git a/pkgs/tools/package-management/nix/patches/ghsa-g948-229j-48j3-2.26.patch b/pkgs/tools/package-management/nix/patches/ghsa-g948-229j-48j3-2.26.patch new file mode 100644 index 000000000000..985e7b29e53f --- /dev/null +++ b/pkgs/tools/package-management/nix/patches/ghsa-g948-229j-48j3-2.26.patch @@ -0,0 +1,463 @@ +From 787e012f26761e1455e711ab4ceedaa2c740621c Mon Sep 17 00:00:00 2001 +From: Eelco Dolstra +Date: Thu, 19 Jun 2025 16:20:34 +0200 +Subject: [PATCH] Fixes for GHSA-g948-229j-48j3 +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Squashed commit of the following: + +commit 04fff3a637d455cbb1d75937a235950e43008db9 +Author: Eelco Dolstra +Date: Thu Jun 12 12:30:32 2025 +0200 + + Chown structured attr files safely + +commit 5417ad445e414c649d0cfc71a05661c7bf8f3ef5 +Author: Eelco Dolstra +Date: Thu Jun 12 12:14:04 2025 +0200 + + Replace 'bool sync' with an enum for clarity + + And drop writeFileAndSync(). + +commit 7ae0141f328d8e8e1094be24665789c05f974ba6 +Author: Eelco Dolstra +Date: Thu Jun 12 11:35:28 2025 +0200 + + Drop guessOrInventPathFromFD() + + No need to do hacky stuff like that when we already know the original path. + +commit 45b05098bd019da7c57cd4227a89bfd0fa65bb08 +Author: Eelco Dolstra +Date: Thu Jun 12 11:15:58 2025 +0200 + + Tweak comment + +commit 0af15b31209d1b7ec8addfae9a1a6b60d8f35848 +Author: Raito Bezarius +Date: Thu Mar 27 12:22:26 2025 +0100 + + libstore: ensure that temporary directory is always 0o000 before deletion + + In the case the deletion fails, we should ensure that the temporary + directory cannot be used for nefarious purposes. + + Change-Id: I498a2dd0999a74195d13642f44a5de1e69d46120 + Signed-off-by: Raito Bezarius + +commit 2c20fa37b15cfa03ac6a1a6a47cdb2ed66c0827e +Author: Raito Bezarius +Date: Wed Mar 26 12:42:55 2025 +0100 + + libutil: ensure that `_deletePath` does NOT use absolute paths with dirfds + + When calling `_deletePath` with a parent file descriptor, `openat` is + made effective by using relative paths to the directory file descriptor. + + To avoid the problem, the signature is changed to resist misuse with an + assert in the prologue of the function. + + Change-Id: I6b3fc766bad2afe54dc27d47d1df3873e188de96 + Signed-off-by: Raito Bezarius + +commit d3c370bbcae48bb825ce19fd0f73bb4eefd2c9ea +Author: Raito Bezarius +Date: Wed Mar 26 01:07:47 2025 +0100 + + libstore: ensure that `passAsFile` is created in the original temp dir + + This ensures that `passAsFile` data is created inside the expected + temporary build directory by `openat()` from the parent directory file + descriptor. + + This avoids a TOCTOU which is part of the attack chain of CVE-????. + + Change-Id: Ie5273446c4a19403088d0389ae8e3f473af8879a + Signed-off-by: Raito Bezarius + +commit 45d3598724f932d024ef6bc2ffb00c1bb90e6018 +Author: Raito Bezarius +Date: Wed Mar 26 01:06:03 2025 +0100 + + libutil: writeFile variant for file descriptors + + `writeFile` lose its `sync` boolean flag to make things simpler. + + A new `writeFileAndSync` function is created and all call sites are + converted to it. + + Change-Id: Ib871a5283a9c047db1e4fe48a241506e4aab9192 + Signed-off-by: Raito Bezarius + +commit 732bd9b98cabf4aaf95a01fd318923de303f9996 +Author: Raito Bezarius +Date: Wed Mar 26 01:05:34 2025 +0100 + + libstore: chown to builder variant for file descriptors + + We use it immediately for the build temporary directory. + + Change-Id: I180193c63a2b98721f5fb8e542c4e39c099bb947 + Signed-off-by: Raito Bezarius + +commit 962c65f8dcd5570dd92c72370a862c7b38942e0d +Author: Raito Bezarius +Date: Wed Mar 26 01:04:59 2025 +0100 + + libstore: open build directory as a dirfd as well + + We now keep around a proper AutoCloseFD around the temporary directory + which we plan to use for openat operations and avoiding the build + directory being swapped out while we are doing something else. + + Change-Id: I18d387b0f123ebf2d20c6405cd47ebadc5505f2a + Signed-off-by: Raito Bezarius + +commit c9b42462b75b5a37ee6564c2b53cff186c8323da +Author: Raito Bezarius +Date: Wed Mar 26 01:04:12 2025 +0100 + + libutil: guess or invent a path from file descriptors + + This is useful for certain error recovery paths (no pun intended) that + does not thread through the original path name. + + Change-Id: I2d800740cb4f9912e64c923120d3f977c58ccb7e + Signed-off-by: Raito Bezarius + +Signed-off-by: Jörg Thalheim +--- + src/libstore/local-store.cc | 6 +-- + .../unix/build/local-derivation-goal.cc | 46 ++++++++++++++---- + .../unix/build/local-derivation-goal.hh | 20 ++++++++ + src/libutil/file-content-address.cc | 2 +- + src/libutil/file-system.cc | 47 +++++++++++-------- + src/libutil/file-system.hh | 14 ++++-- + 6 files changed, 99 insertions(+), 36 deletions(-) + +diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc +index 9a7a941b6..c0c808e0a 100644 +--- a/src/libstore/local-store.cc ++++ b/src/libstore/local-store.cc +@@ -116,7 +116,7 @@ LocalStore::LocalStore( + state->stmts = std::make_unique(); + + /* Create missing state directories if they don't already exist. */ +- createDirs(realStoreDir); ++ createDirs(realStoreDir.get()); + if (readOnly) { + experimentalFeatureSettings.require(Xp::ReadOnlyLocalStore); + } else { +@@ -248,7 +248,7 @@ LocalStore::LocalStore( + else if (curSchema == 0) { /* new store */ + curSchema = nixSchemaVersion; + openDB(*state, true); +- writeFile(schemaPath, fmt("%1%", nixSchemaVersion), 0666, true); ++ writeFile(schemaPath, fmt("%1%", curSchema), 0666, FsSync::Yes); + } + + else if (curSchema < nixSchemaVersion) { +@@ -299,7 +299,7 @@ LocalStore::LocalStore( + txn.commit(); + } + +- writeFile(schemaPath, fmt("%1%", nixSchemaVersion), 0666, true); ++ writeFile(schemaPath, fmt("%1%", nixSchemaVersion), 0666, FsSync::Yes); + + lockFile(globalLock.get(), ltRead, true); + } +diff --git a/src/libstore/unix/build/local-derivation-goal.cc b/src/libstore/unix/build/local-derivation-goal.cc +index 5b9bc0bb0..80309e332 100644 +--- a/src/libstore/unix/build/local-derivation-goal.cc ++++ b/src/libstore/unix/build/local-derivation-goal.cc +@@ -559,7 +559,14 @@ void LocalDerivationGoal::startBuilder() + } else { + tmpDir = topTmpDir; + } +- chownToBuilder(tmpDir); ++ ++ /* The TOCTOU between the previous mkdir call and this open call is unavoidable due to ++ POSIX semantics.*/ ++ tmpDirFd = AutoCloseFD{open(tmpDir.c_str(), O_RDONLY | O_NOFOLLOW | O_DIRECTORY)}; ++ if (!tmpDirFd) ++ throw SysError("failed to open the build temporary directory descriptor '%1%'", tmpDir); ++ ++ chownToBuilder(tmpDirFd.get(), tmpDir); + + for (auto & [outputName, status] : initialOutputs) { + /* Set scratch path we'll actually use during the build. +@@ -1157,9 +1164,7 @@ void LocalDerivationGoal::initTmpDir() + } else { + auto hash = hashString(HashAlgorithm::SHA256, i.first); + std::string fn = ".attr-" + hash.to_string(HashFormat::Nix32, false); +- Path p = tmpDir + "/" + fn; +- writeFile(p, rewriteStrings(i.second, inputRewrites)); +- chownToBuilder(p); ++ writeBuilderFile(fn, rewriteStrings(i.second, inputRewrites)); + env[i.first + "Path"] = tmpDirInSandbox + "/" + fn; + } + } +@@ -1264,11 +1269,9 @@ void LocalDerivationGoal::writeStructuredAttrs() + + auto jsonSh = writeStructuredAttrsShell(json); + +- writeFile(tmpDir + "/.attrs.sh", rewriteStrings(jsonSh, inputRewrites)); +- chownToBuilder(tmpDir + "/.attrs.sh"); ++ writeBuilderFile(".attrs.sh", rewriteStrings(jsonSh, inputRewrites)); + env["NIX_ATTRS_SH_FILE"] = tmpDirInSandbox + "/.attrs.sh"; +- writeFile(tmpDir + "/.attrs.json", rewriteStrings(json.dump(), inputRewrites)); +- chownToBuilder(tmpDir + "/.attrs.json"); ++ writeBuilderFile(".attrs.json", rewriteStrings(json.dump(), inputRewrites)); + env["NIX_ATTRS_JSON_FILE"] = tmpDirInSandbox + "/.attrs.json"; + } + } +@@ -1779,6 +1782,24 @@ void setupSeccomp() + #endif + } + ++void LocalDerivationGoal::chownToBuilder(int fd, const Path & path) ++{ ++ if (!buildUser) return; ++ if (fchown(fd, buildUser->getUID(), buildUser->getGID()) == -1) ++ throw SysError("cannot change ownership of file '%1%'", path); ++} ++ ++void LocalDerivationGoal::writeBuilderFile( ++ const std::string & name, ++ std::string_view contents) ++{ ++ auto path = std::filesystem::path(tmpDir) / name; ++ AutoCloseFD fd{openat(tmpDirFd.get(), name.c_str(), O_WRONLY | O_TRUNC | O_CREAT | O_CLOEXEC | O_EXCL | O_NOFOLLOW, 0666)}; ++ if (!fd) ++ throw SysError("creating file %s", path); ++ writeFile(fd, path, contents); ++ chownToBuilder(fd.get(), path); ++} + + void LocalDerivationGoal::runChild() + { +@@ -3038,6 +3059,15 @@ void LocalDerivationGoal::checkOutputs(const std::mapisBuiltin()) { +diff --git a/src/libstore/unix/build/local-derivation-goal.hh b/src/libstore/unix/build/local-derivation-goal.hh +index 1ea247661..74a1e1c50 100644 +--- a/src/libstore/unix/build/local-derivation-goal.hh ++++ b/src/libstore/unix/build/local-derivation-goal.hh +@@ -37,6 +37,11 @@ struct LocalDerivationGoal : public DerivationGoal + */ + Path topTmpDir; + ++ /** ++ * The file descriptor of the temporary directory. ++ */ ++ AutoCloseFD tmpDirFd; ++ + /** + * The path of the temporary directory in the sandbox. + */ +@@ -244,9 +249,24 @@ struct LocalDerivationGoal : public DerivationGoal + + /** + * Make a file owned by the builder. ++ * ++ * SAFETY: this function is prone to TOCTOU as it receives a path and not a descriptor. ++ * It's only safe to call in a child of a directory only visible to the owner. + */ + void chownToBuilder(const Path & path); + ++ /** ++ * Make a file owned by the builder addressed by its file descriptor. ++ */ ++ void chownToBuilder(int fd, const Path & path); ++ ++ /** ++ * Create a file in `tmpDir` owned by the builder. ++ */ ++ void writeBuilderFile( ++ const std::string & name, ++ std::string_view contents); ++ + int getChildStatus() override; + + /** +diff --git a/src/libutil/file-content-address.cc b/src/libutil/file-content-address.cc +index 69301d9c8..2b6839346 100644 +--- a/src/libutil/file-content-address.cc ++++ b/src/libutil/file-content-address.cc +@@ -93,7 +93,7 @@ void restorePath( + { + switch (method) { + case FileSerialisationMethod::Flat: +- writeFile(path, source, 0666, startFsync); ++ writeFile(path, source, 0666, startFsync ? FsSync::Yes : FsSync::No); + break; + case FileSerialisationMethod::NixArchive: + restorePath(path, source, startFsync); +diff --git a/src/libutil/file-system.cc b/src/libutil/file-system.cc +index 6fe93b63a..b3183f495 100644 +--- a/src/libutil/file-system.cc ++++ b/src/libutil/file-system.cc +@@ -258,7 +258,7 @@ void readFile(const Path & path, Sink & sink) + } + + +-void writeFile(const Path & path, std::string_view s, mode_t mode, bool sync) ++void writeFile(const Path & path, std::string_view s, mode_t mode, FsSync sync) + { + AutoCloseFD fd = toDescriptor(open(path.c_str(), O_WRONLY | O_TRUNC | O_CREAT + // TODO +@@ -268,22 +268,29 @@ void writeFile(const Path & path, std::string_view s, mode_t mode, bool sync) + , mode)); + if (!fd) + throw SysError("opening file '%1%'", path); ++ ++ writeFile(fd, path, s, mode, sync); ++ ++ /* Close explicitly to propagate the exceptions. */ ++ fd.close(); ++} ++ ++void writeFile(AutoCloseFD & fd, const Path & origPath, std::string_view s, mode_t mode, FsSync sync) ++{ ++ assert(fd); + try { + writeFull(fd.get(), s); ++ ++ if (sync == FsSync::Yes) ++ fd.fsync(); ++ + } catch (Error & e) { +- e.addTrace({}, "writing file '%1%'", path); ++ e.addTrace({}, "writing file '%1%'", origPath); + throw; + } +- if (sync) +- fd.fsync(); +- // Explicitly close to make sure exceptions are propagated. +- fd.close(); +- if (sync) +- syncParent(path); + } + +- +-void writeFile(const Path & path, Source & source, mode_t mode, bool sync) ++void writeFile(const Path & path, Source & source, mode_t mode, FsSync sync) + { + AutoCloseFD fd = toDescriptor(open(path.c_str(), O_WRONLY | O_TRUNC | O_CREAT + // TODO +@@ -307,11 +314,11 @@ void writeFile(const Path & path, Source & source, mode_t mode, bool sync) + e.addTrace({}, "writing file '%1%'", path); + throw; + } +- if (sync) ++ if (sync == FsSync::Yes) + fd.fsync(); + // Explicitly close to make sure exceptions are propagated. + fd.close(); +- if (sync) ++ if (sync == FsSync::Yes) + syncParent(path); + } + +@@ -374,7 +381,8 @@ static void _deletePath(Descriptor parentfd, const fs::path & path, uint64_t & b + #ifndef _WIN32 + checkInterrupt(); + +- std::string name(baseNameOf(path.native())); ++ std::string name(path.filename()); ++ assert(name != "." && name != ".." && !name.empty()); + + struct stat st; + if (fstatat(parentfd, name.c_str(), &st, +@@ -415,7 +423,7 @@ static void _deletePath(Descriptor parentfd, const fs::path & path, uint64_t & b + throw SysError("chmod %1%", path); + } + +- int fd = openat(parentfd, path.c_str(), O_RDONLY); ++ int fd = openat(parentfd, name.c_str(), O_RDONLY | O_DIRECTORY | O_NOFOLLOW); + if (fd == -1) + throw SysError("opening directory %1%", path); + AutoCloseDir dir(fdopendir(fd)); +@@ -427,7 +435,7 @@ static void _deletePath(Descriptor parentfd, const fs::path & path, uint64_t & b + checkInterrupt(); + std::string childName = dirent->d_name; + if (childName == "." || childName == "..") continue; +- _deletePath(dirfd(dir.get()), path + "/" + childName, bytesFreed); ++ _deletePath(dirfd(dir.get()), path / childName, bytesFreed); + } + if (errno) throw SysError("reading directory %1%", path); + } +@@ -445,14 +453,13 @@ static void _deletePath(Descriptor parentfd, const fs::path & path, uint64_t & b + + static void _deletePath(const fs::path & path, uint64_t & bytesFreed) + { +- Path dir = dirOf(path.string()); +- if (dir == "") +- dir = "/"; ++ assert(path.is_absolute()); ++ assert(path.parent_path() != path); + +- AutoCloseFD dirfd = toDescriptor(open(dir.c_str(), O_RDONLY)); ++ AutoCloseFD dirfd = toDescriptor(open(path.parent_path().string().c_str(), O_RDONLY)); + if (!dirfd) { + if (errno == ENOENT) return; +- throw SysError("opening directory '%1%'", path); ++ throw SysError("opening directory %s", path.parent_path()); + } + + _deletePath(dirfd.get(), path, bytesFreed); +diff --git a/src/libutil/file-system.hh b/src/libutil/file-system.hh +index 204907339..b2db8869e 100644 +--- a/src/libutil/file-system.hh ++++ b/src/libutil/file-system.hh +@@ -194,21 +194,27 @@ std::string readFile(const Path & path); + std::string readFile(const std::filesystem::path & path); + void readFile(const Path & path, Sink & sink); + ++enum struct FsSync { Yes, No }; ++ + /** + * Write a string to a file. + */ +-void writeFile(const Path & path, std::string_view s, mode_t mode = 0666, bool sync = false); +-static inline void writeFile(const std::filesystem::path & path, std::string_view s, mode_t mode = 0666, bool sync = false) ++void writeFile(const Path & path, std::string_view s, mode_t mode = 0666, FsSync sync = FsSync::No); ++ ++static inline void writeFile(const std::filesystem::path & path, std::string_view s, mode_t mode = 0666, FsSync sync = FsSync::No) + { + return writeFile(path.string(), s, mode, sync); + } + +-void writeFile(const Path & path, Source & source, mode_t mode = 0666, bool sync = false); +-static inline void writeFile(const std::filesystem::path & path, Source & source, mode_t mode = 0666, bool sync = false) ++void writeFile(const Path & path, Source & source, mode_t mode = 0666, FsSync sync = FsSync::No); ++ ++static inline void writeFile(const std::filesystem::path & path, Source & source, mode_t mode = 0666, FsSync sync = FsSync::No) + { + return writeFile(path.string(), source, mode, sync); + } + ++void writeFile(AutoCloseFD & fd, const Path & origPath, std::string_view s, mode_t mode = 0666, FsSync sync = FsSync::No); ++ + /** + * Flush a path's parent directory to disk. + */ +-- +2.49.0 + diff --git a/pkgs/tools/package-management/nix/patches/ghsa-g948-229j-48j3-2.28.patch b/pkgs/tools/package-management/nix/patches/ghsa-g948-229j-48j3-2.28.patch new file mode 100644 index 000000000000..7de531128279 --- /dev/null +++ b/pkgs/tools/package-management/nix/patches/ghsa-g948-229j-48j3-2.28.patch @@ -0,0 +1,454 @@ +From 24c1aa735a40d3bf5361755fa10ac0e577a55eed Mon Sep 17 00:00:00 2001 +From: Eelco Dolstra +Date: Thu, 19 Jun 2025 16:20:34 +0200 +Subject: [PATCH] Fixes for GHSA-g948-229j-48j3 +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Squashed commit of the following: + +commit 04fff3a637d455cbb1d75937a235950e43008db9 +Author: Eelco Dolstra +Date: Thu Jun 12 12:30:32 2025 +0200 + + Chown structured attr files safely + +commit 5417ad445e414c649d0cfc71a05661c7bf8f3ef5 +Author: Eelco Dolstra +Date: Thu Jun 12 12:14:04 2025 +0200 + + Replace 'bool sync' with an enum for clarity + + And drop writeFileAndSync(). + +commit 7ae0141f328d8e8e1094be24665789c05f974ba6 +Author: Eelco Dolstra +Date: Thu Jun 12 11:35:28 2025 +0200 + + Drop guessOrInventPathFromFD() + + No need to do hacky stuff like that when we already know the original path. + +commit 45b05098bd019da7c57cd4227a89bfd0fa65bb08 +Author: Eelco Dolstra +Date: Thu Jun 12 11:15:58 2025 +0200 + + Tweak comment + +commit 0af15b31209d1b7ec8addfae9a1a6b60d8f35848 +Author: Raito Bezarius +Date: Thu Mar 27 12:22:26 2025 +0100 + + libstore: ensure that temporary directory is always 0o000 before deletion + + In the case the deletion fails, we should ensure that the temporary + directory cannot be used for nefarious purposes. + + Change-Id: I498a2dd0999a74195d13642f44a5de1e69d46120 + Signed-off-by: Raito Bezarius + +commit 2c20fa37b15cfa03ac6a1a6a47cdb2ed66c0827e +Author: Raito Bezarius +Date: Wed Mar 26 12:42:55 2025 +0100 + + libutil: ensure that `_deletePath` does NOT use absolute paths with dirfds + + When calling `_deletePath` with a parent file descriptor, `openat` is + made effective by using relative paths to the directory file descriptor. + + To avoid the problem, the signature is changed to resist misuse with an + assert in the prologue of the function. + + Change-Id: I6b3fc766bad2afe54dc27d47d1df3873e188de96 + Signed-off-by: Raito Bezarius + +commit d3c370bbcae48bb825ce19fd0f73bb4eefd2c9ea +Author: Raito Bezarius +Date: Wed Mar 26 01:07:47 2025 +0100 + + libstore: ensure that `passAsFile` is created in the original temp dir + + This ensures that `passAsFile` data is created inside the expected + temporary build directory by `openat()` from the parent directory file + descriptor. + + This avoids a TOCTOU which is part of the attack chain of CVE-????. + + Change-Id: Ie5273446c4a19403088d0389ae8e3f473af8879a + Signed-off-by: Raito Bezarius + +commit 45d3598724f932d024ef6bc2ffb00c1bb90e6018 +Author: Raito Bezarius +Date: Wed Mar 26 01:06:03 2025 +0100 + + libutil: writeFile variant for file descriptors + + `writeFile` lose its `sync` boolean flag to make things simpler. + + A new `writeFileAndSync` function is created and all call sites are + converted to it. + + Change-Id: Ib871a5283a9c047db1e4fe48a241506e4aab9192 + Signed-off-by: Raito Bezarius + +commit 732bd9b98cabf4aaf95a01fd318923de303f9996 +Author: Raito Bezarius +Date: Wed Mar 26 01:05:34 2025 +0100 + + libstore: chown to builder variant for file descriptors + + We use it immediately for the build temporary directory. + + Change-Id: I180193c63a2b98721f5fb8e542c4e39c099bb947 + Signed-off-by: Raito Bezarius + +commit 962c65f8dcd5570dd92c72370a862c7b38942e0d +Author: Raito Bezarius +Date: Wed Mar 26 01:04:59 2025 +0100 + + libstore: open build directory as a dirfd as well + + We now keep around a proper AutoCloseFD around the temporary directory + which we plan to use for openat operations and avoiding the build + directory being swapped out while we are doing something else. + + Change-Id: I18d387b0f123ebf2d20c6405cd47ebadc5505f2a + Signed-off-by: Raito Bezarius + +commit c9b42462b75b5a37ee6564c2b53cff186c8323da +Author: Raito Bezarius +Date: Wed Mar 26 01:04:12 2025 +0100 + + libutil: guess or invent a path from file descriptors + + This is useful for certain error recovery paths (no pun intended) that + does not thread through the original path name. + + Change-Id: I2d800740cb4f9912e64c923120d3f977c58ccb7e + Signed-off-by: Raito Bezarius + +Signed-off-by: Jörg Thalheim +--- + src/libstore/local-store.cc | 4 +- + .../unix/build/local-derivation-goal.cc | 46 ++++++++++++++---- + .../nix/store/build/local-derivation-goal.hh | 20 ++++++++ + src/libutil/file-content-address.cc | 2 +- + src/libutil/file-system.cc | 47 +++++++++++-------- + src/libutil/include/nix/util/file-system.hh | 14 ++++-- + 6 files changed, 98 insertions(+), 35 deletions(-) + +diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc +index f3bee6953..eddc87ef9 100644 +--- a/src/libstore/local-store.cc ++++ b/src/libstore/local-store.cc +@@ -249,7 +249,7 @@ LocalStore::LocalStore( + else if (curSchema == 0) { /* new store */ + curSchema = nixSchemaVersion; + openDB(*state, true); +- writeFile(schemaPath, fmt("%1%", curSchema), 0666, true); ++ writeFile(schemaPath, fmt("%1%", curSchema), 0666, FsSync::Yes); + } + + else if (curSchema < nixSchemaVersion) { +@@ -300,7 +300,7 @@ LocalStore::LocalStore( + txn.commit(); + } + +- writeFile(schemaPath, fmt("%1%", nixSchemaVersion), 0666, true); ++ writeFile(schemaPath, fmt("%1%", nixSchemaVersion), 0666, FsSync::Yes); + + lockFile(globalLock.get(), ltRead, true); + } +diff --git a/src/libstore/unix/build/local-derivation-goal.cc b/src/libstore/unix/build/local-derivation-goal.cc +index 9edb6fb0f..a0442d0b8 100644 +--- a/src/libstore/unix/build/local-derivation-goal.cc ++++ b/src/libstore/unix/build/local-derivation-goal.cc +@@ -567,7 +567,14 @@ void LocalDerivationGoal::startBuilder() + } else { + tmpDir = topTmpDir; + } +- chownToBuilder(tmpDir); ++ ++ /* The TOCTOU between the previous mkdir call and this open call is unavoidable due to ++ POSIX semantics.*/ ++ tmpDirFd = AutoCloseFD{open(tmpDir.c_str(), O_RDONLY | O_NOFOLLOW | O_DIRECTORY)}; ++ if (!tmpDirFd) ++ throw SysError("failed to open the build temporary directory descriptor '%1%'", tmpDir); ++ ++ chownToBuilder(tmpDirFd.get(), tmpDir); + + for (auto & [outputName, status] : initialOutputs) { + /* Set scratch path we'll actually use during the build. +@@ -1159,9 +1166,7 @@ void LocalDerivationGoal::initTmpDir() + } else { + auto hash = hashString(HashAlgorithm::SHA256, i.first); + std::string fn = ".attr-" + hash.to_string(HashFormat::Nix32, false); +- Path p = tmpDir + "/" + fn; +- writeFile(p, rewriteStrings(i.second, inputRewrites)); +- chownToBuilder(p); ++ writeBuilderFile(fn, rewriteStrings(i.second, inputRewrites)); + env[i.first + "Path"] = tmpDirInSandbox + "/" + fn; + } + } +@@ -1266,11 +1271,9 @@ void LocalDerivationGoal::writeStructuredAttrs() + + auto jsonSh = writeStructuredAttrsShell(json); + +- writeFile(tmpDir + "/.attrs.sh", rewriteStrings(jsonSh, inputRewrites)); +- chownToBuilder(tmpDir + "/.attrs.sh"); ++ writeBuilderFile(".attrs.sh", rewriteStrings(jsonSh, inputRewrites)); + env["NIX_ATTRS_SH_FILE"] = tmpDirInSandbox + "/.attrs.sh"; +- writeFile(tmpDir + "/.attrs.json", rewriteStrings(json.dump(), inputRewrites)); +- chownToBuilder(tmpDir + "/.attrs.json"); ++ writeBuilderFile(".attrs.json", rewriteStrings(json.dump(), inputRewrites)); + env["NIX_ATTRS_JSON_FILE"] = tmpDirInSandbox + "/.attrs.json"; + } + } +@@ -1781,6 +1784,24 @@ void setupSeccomp() + #endif + } + ++void LocalDerivationGoal::chownToBuilder(int fd, const Path & path) ++{ ++ if (!buildUser) return; ++ if (fchown(fd, buildUser->getUID(), buildUser->getGID()) == -1) ++ throw SysError("cannot change ownership of file '%1%'", path); ++} ++ ++void LocalDerivationGoal::writeBuilderFile( ++ const std::string & name, ++ std::string_view contents) ++{ ++ auto path = std::filesystem::path(tmpDir) / name; ++ AutoCloseFD fd{openat(tmpDirFd.get(), name.c_str(), O_WRONLY | O_TRUNC | O_CREAT | O_CLOEXEC | O_EXCL | O_NOFOLLOW, 0666)}; ++ if (!fd) ++ throw SysError("creating file %s", path); ++ writeFile(fd, path, contents); ++ chownToBuilder(fd.get(), path); ++} + + void LocalDerivationGoal::runChild() + { +@@ -3000,6 +3021,15 @@ void LocalDerivationGoal::checkOutputs(const std::mapisBuiltin()) { +diff --git a/src/libstore/unix/include/nix/store/build/local-derivation-goal.hh b/src/libstore/unix/include/nix/store/build/local-derivation-goal.hh +index 795286a01..fb62e3ca4 100644 +--- a/src/libstore/unix/include/nix/store/build/local-derivation-goal.hh ++++ b/src/libstore/unix/include/nix/store/build/local-derivation-goal.hh +@@ -37,6 +37,11 @@ struct LocalDerivationGoal : public DerivationGoal + */ + Path topTmpDir; + ++ /** ++ * The file descriptor of the temporary directory. ++ */ ++ AutoCloseFD tmpDirFd; ++ + /** + * The path of the temporary directory in the sandbox. + */ +@@ -239,9 +244,24 @@ struct LocalDerivationGoal : public DerivationGoal + + /** + * Make a file owned by the builder. ++ * ++ * SAFETY: this function is prone to TOCTOU as it receives a path and not a descriptor. ++ * It's only safe to call in a child of a directory only visible to the owner. + */ + void chownToBuilder(const Path & path); + ++ /** ++ * Make a file owned by the builder addressed by its file descriptor. ++ */ ++ void chownToBuilder(int fd, const Path & path); ++ ++ /** ++ * Create a file in `tmpDir` owned by the builder. ++ */ ++ void writeBuilderFile( ++ const std::string & name, ++ std::string_view contents); ++ + int getChildStatus() override; + + /** +diff --git a/src/libutil/file-content-address.cc b/src/libutil/file-content-address.cc +index 142bc70d5..d95781691 100644 +--- a/src/libutil/file-content-address.cc ++++ b/src/libutil/file-content-address.cc +@@ -93,7 +93,7 @@ void restorePath( + { + switch (method) { + case FileSerialisationMethod::Flat: +- writeFile(path, source, 0666, startFsync); ++ writeFile(path, source, 0666, startFsync ? FsSync::Yes : FsSync::No); + break; + case FileSerialisationMethod::NixArchive: + restorePath(path, source, startFsync); +diff --git a/src/libutil/file-system.cc b/src/libutil/file-system.cc +index 9ce3682f1..204a63c4e 100644 +--- a/src/libutil/file-system.cc ++++ b/src/libutil/file-system.cc +@@ -298,7 +298,7 @@ void readFile(const Path & path, Sink & sink) + } + + +-void writeFile(const Path & path, std::string_view s, mode_t mode, bool sync) ++void writeFile(const Path & path, std::string_view s, mode_t mode, FsSync sync) + { + AutoCloseFD fd = toDescriptor(open(path.c_str(), O_WRONLY | O_TRUNC | O_CREAT + // TODO +@@ -308,22 +308,29 @@ void writeFile(const Path & path, std::string_view s, mode_t mode, bool sync) + , mode)); + if (!fd) + throw SysError("opening file '%1%'", path); ++ ++ writeFile(fd, path, s, mode, sync); ++ ++ /* Close explicitly to propagate the exceptions. */ ++ fd.close(); ++} ++ ++void writeFile(AutoCloseFD & fd, const Path & origPath, std::string_view s, mode_t mode, FsSync sync) ++{ ++ assert(fd); + try { + writeFull(fd.get(), s); ++ ++ if (sync == FsSync::Yes) ++ fd.fsync(); ++ + } catch (Error & e) { +- e.addTrace({}, "writing file '%1%'", path); ++ e.addTrace({}, "writing file '%1%'", origPath); + throw; + } +- if (sync) +- fd.fsync(); +- // Explicitly close to make sure exceptions are propagated. +- fd.close(); +- if (sync) +- syncParent(path); + } + +- +-void writeFile(const Path & path, Source & source, mode_t mode, bool sync) ++void writeFile(const Path & path, Source & source, mode_t mode, FsSync sync) + { + AutoCloseFD fd = toDescriptor(open(path.c_str(), O_WRONLY | O_TRUNC | O_CREAT + // TODO +@@ -347,11 +354,11 @@ void writeFile(const Path & path, Source & source, mode_t mode, bool sync) + e.addTrace({}, "writing file '%1%'", path); + throw; + } +- if (sync) ++ if (sync == FsSync::Yes) + fd.fsync(); + // Explicitly close to make sure exceptions are propagated. + fd.close(); +- if (sync) ++ if (sync == FsSync::Yes) + syncParent(path); + } + +@@ -414,7 +421,8 @@ static void _deletePath(Descriptor parentfd, const fs::path & path, uint64_t & b + #ifndef _WIN32 + checkInterrupt(); + +- std::string name(baseNameOf(path.native())); ++ std::string name(path.filename()); ++ assert(name != "." && name != ".." && !name.empty()); + + struct stat st; + if (fstatat(parentfd, name.c_str(), &st, +@@ -455,7 +463,7 @@ static void _deletePath(Descriptor parentfd, const fs::path & path, uint64_t & b + throw SysError("chmod %1%", path); + } + +- int fd = openat(parentfd, path.c_str(), O_RDONLY); ++ int fd = openat(parentfd, name.c_str(), O_RDONLY | O_DIRECTORY | O_NOFOLLOW); + if (fd == -1) + throw SysError("opening directory %1%", path); + AutoCloseDir dir(fdopendir(fd)); +@@ -467,7 +475,7 @@ static void _deletePath(Descriptor parentfd, const fs::path & path, uint64_t & b + checkInterrupt(); + std::string childName = dirent->d_name; + if (childName == "." || childName == "..") continue; +- _deletePath(dirfd(dir.get()), path + "/" + childName, bytesFreed); ++ _deletePath(dirfd(dir.get()), path / childName, bytesFreed); + } + if (errno) throw SysError("reading directory %1%", path); + } +@@ -485,14 +493,13 @@ static void _deletePath(Descriptor parentfd, const fs::path & path, uint64_t & b + + static void _deletePath(const fs::path & path, uint64_t & bytesFreed) + { +- Path dir = dirOf(path.string()); +- if (dir == "") +- dir = "/"; ++ assert(path.is_absolute()); ++ assert(path.parent_path() != path); + +- AutoCloseFD dirfd = toDescriptor(open(dir.c_str(), O_RDONLY)); ++ AutoCloseFD dirfd = toDescriptor(open(path.parent_path().string().c_str(), O_RDONLY)); + if (!dirfd) { + if (errno == ENOENT) return; +- throw SysError("opening directory '%1%'", path); ++ throw SysError("opening directory %s", path.parent_path()); + } + + _deletePath(dirfd.get(), path, bytesFreed); +diff --git a/src/libutil/include/nix/util/file-system.hh b/src/libutil/include/nix/util/file-system.hh +index e6b1cfef3..9a0057bbe 100644 +--- a/src/libutil/include/nix/util/file-system.hh ++++ b/src/libutil/include/nix/util/file-system.hh +@@ -193,21 +193,27 @@ std::string readFile(const Path & path); + std::string readFile(const std::filesystem::path & path); + void readFile(const Path & path, Sink & sink); + ++enum struct FsSync { Yes, No }; ++ + /** + * Write a string to a file. + */ +-void writeFile(const Path & path, std::string_view s, mode_t mode = 0666, bool sync = false); +-static inline void writeFile(const std::filesystem::path & path, std::string_view s, mode_t mode = 0666, bool sync = false) ++void writeFile(const Path & path, std::string_view s, mode_t mode = 0666, FsSync sync = FsSync::No); ++ ++static inline void writeFile(const std::filesystem::path & path, std::string_view s, mode_t mode = 0666, FsSync sync = FsSync::No) + { + return writeFile(path.string(), s, mode, sync); + } + +-void writeFile(const Path & path, Source & source, mode_t mode = 0666, bool sync = false); +-static inline void writeFile(const std::filesystem::path & path, Source & source, mode_t mode = 0666, bool sync = false) ++void writeFile(const Path & path, Source & source, mode_t mode = 0666, FsSync sync = FsSync::No); ++ ++static inline void writeFile(const std::filesystem::path & path, Source & source, mode_t mode = 0666, FsSync sync = FsSync::No) + { + return writeFile(path.string(), source, mode, sync); + } + ++void writeFile(AutoCloseFD & fd, const Path & origPath, std::string_view s, mode_t mode = 0666, FsSync sync = FsSync::No); ++ + /** + * Flush a path's parent directory to disk. + */ +-- +2.44.1 + diff --git a/pkgs/tools/package-management/nix/patches/ghsa-g948-229j-48j3-2.29.patch b/pkgs/tools/package-management/nix/patches/ghsa-g948-229j-48j3-2.29.patch new file mode 100644 index 000000000000..265c2580cfe7 --- /dev/null +++ b/pkgs/tools/package-management/nix/patches/ghsa-g948-229j-48j3-2.29.patch @@ -0,0 +1,449 @@ +From 01619fbe2dc06b79609b95b6f95ddbf4e871e762 Mon Sep 17 00:00:00 2001 +From: Eelco Dolstra +Date: Thu, 19 Jun 2025 16:20:34 +0200 +Subject: [PATCH] Fixes for GHSA-g948-229j-48j3 +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Squashed commit of the following: + +commit 04fff3a637d455cbb1d75937a235950e43008db9 +Author: Eelco Dolstra +Date: Thu Jun 12 12:30:32 2025 +0200 + + Chown structured attr files safely + +commit 5417ad445e414c649d0cfc71a05661c7bf8f3ef5 +Author: Eelco Dolstra +Date: Thu Jun 12 12:14:04 2025 +0200 + + Replace 'bool sync' with an enum for clarity + + And drop writeFileAndSync(). + +commit 7ae0141f328d8e8e1094be24665789c05f974ba6 +Author: Eelco Dolstra +Date: Thu Jun 12 11:35:28 2025 +0200 + + Drop guessOrInventPathFromFD() + + No need to do hacky stuff like that when we already know the original path. + +commit 45b05098bd019da7c57cd4227a89bfd0fa65bb08 +Author: Eelco Dolstra +Date: Thu Jun 12 11:15:58 2025 +0200 + + Tweak comment + +commit 0af15b31209d1b7ec8addfae9a1a6b60d8f35848 +Author: Raito Bezarius +Date: Thu Mar 27 12:22:26 2025 +0100 + + libstore: ensure that temporary directory is always 0o000 before deletion + + In the case the deletion fails, we should ensure that the temporary + directory cannot be used for nefarious purposes. + + Change-Id: I498a2dd0999a74195d13642f44a5de1e69d46120 + Signed-off-by: Raito Bezarius + +commit 2c20fa37b15cfa03ac6a1a6a47cdb2ed66c0827e +Author: Raito Bezarius +Date: Wed Mar 26 12:42:55 2025 +0100 + + libutil: ensure that `_deletePath` does NOT use absolute paths with dirfds + + When calling `_deletePath` with a parent file descriptor, `openat` is + made effective by using relative paths to the directory file descriptor. + + To avoid the problem, the signature is changed to resist misuse with an + assert in the prologue of the function. + + Change-Id: I6b3fc766bad2afe54dc27d47d1df3873e188de96 + Signed-off-by: Raito Bezarius + +commit d3c370bbcae48bb825ce19fd0f73bb4eefd2c9ea +Author: Raito Bezarius +Date: Wed Mar 26 01:07:47 2025 +0100 + + libstore: ensure that `passAsFile` is created in the original temp dir + + This ensures that `passAsFile` data is created inside the expected + temporary build directory by `openat()` from the parent directory file + descriptor. + + This avoids a TOCTOU which is part of the attack chain of CVE-????. + + Change-Id: Ie5273446c4a19403088d0389ae8e3f473af8879a + Signed-off-by: Raito Bezarius + +commit 45d3598724f932d024ef6bc2ffb00c1bb90e6018 +Author: Raito Bezarius +Date: Wed Mar 26 01:06:03 2025 +0100 + + libutil: writeFile variant for file descriptors + + `writeFile` lose its `sync` boolean flag to make things simpler. + + A new `writeFileAndSync` function is created and all call sites are + converted to it. + + Change-Id: Ib871a5283a9c047db1e4fe48a241506e4aab9192 + Signed-off-by: Raito Bezarius + +commit 732bd9b98cabf4aaf95a01fd318923de303f9996 +Author: Raito Bezarius +Date: Wed Mar 26 01:05:34 2025 +0100 + + libstore: chown to builder variant for file descriptors + + We use it immediately for the build temporary directory. + + Change-Id: I180193c63a2b98721f5fb8e542c4e39c099bb947 + Signed-off-by: Raito Bezarius + +commit 962c65f8dcd5570dd92c72370a862c7b38942e0d +Author: Raito Bezarius +Date: Wed Mar 26 01:04:59 2025 +0100 + + libstore: open build directory as a dirfd as well + + We now keep around a proper AutoCloseFD around the temporary directory + which we plan to use for openat operations and avoiding the build + directory being swapped out while we are doing something else. + + Change-Id: I18d387b0f123ebf2d20c6405cd47ebadc5505f2a + Signed-off-by: Raito Bezarius + +commit c9b42462b75b5a37ee6564c2b53cff186c8323da +Author: Raito Bezarius +Date: Wed Mar 26 01:04:12 2025 +0100 + + libutil: guess or invent a path from file descriptors + + This is useful for certain error recovery paths (no pun intended) that + does not thread through the original path name. + + Change-Id: I2d800740cb4f9912e64c923120d3f977c58ccb7e + Signed-off-by: Raito Bezarius + +Signed-off-by: Jörg Thalheim +--- + src/libstore/local-store.cc | 4 +- + src/libstore/unix/build/derivation-builder.cc | 66 ++++++++++++++++--- + src/libutil/file-content-address.cc | 2 +- + src/libutil/file-system.cc | 47 +++++++------ + src/libutil/include/nix/util/file-system.hh | 14 ++-- + 5 files changed, 98 insertions(+), 35 deletions(-) + +diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc +index 76fadba86..1ab3ed13a 100644 +--- a/src/libstore/local-store.cc ++++ b/src/libstore/local-store.cc +@@ -247,7 +247,7 @@ LocalStore::LocalStore(ref config) + else if (curSchema == 0) { /* new store */ + curSchema = nixSchemaVersion; + openDB(*state, true); +- writeFile(schemaPath, fmt("%1%", curSchema), 0666, true); ++ writeFile(schemaPath, fmt("%1%", curSchema), 0666, FsSync::Yes); + } + + else if (curSchema < nixSchemaVersion) { +@@ -298,7 +298,7 @@ LocalStore::LocalStore(ref config) + txn.commit(); + } + +- writeFile(schemaPath, fmt("%1%", nixSchemaVersion), 0666, true); ++ writeFile(schemaPath, fmt("%1%", nixSchemaVersion), 0666, FsSync::Yes); + + lockFile(globalLock.get(), ltRead, true); + } +diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc +index 58e8d8ba6..856bc81c3 100644 +--- a/src/libstore/unix/build/derivation-builder.cc ++++ b/src/libstore/unix/build/derivation-builder.cc +@@ -129,6 +129,11 @@ private: + */ + Path topTmpDir; + ++ /** ++ * The file descriptor of the temporary directory. ++ */ ++ AutoCloseFD tmpDirFd; ++ + /** + * The path of the temporary directory in the sandbox. + */ +@@ -325,9 +330,24 @@ private: + + /** + * Make a file owned by the builder. ++ * ++ * SAFETY: this function is prone to TOCTOU as it receives a path and not a descriptor. ++ * It's only safe to call in a child of a directory only visible to the owner. + */ + void chownToBuilder(const Path & path); + ++ /** ++ * Make a file owned by the builder addressed by its file descriptor. ++ */ ++ void chownToBuilder(int fd, const Path & path); ++ ++ /** ++ * Create a file in `tmpDir` owned by the builder. ++ */ ++ void writeBuilderFile( ++ const std::string & name, ++ std::string_view contents); ++ + /** + * Run the builder's process. + */ +@@ -895,7 +915,14 @@ void DerivationBuilderImpl::startBuilder() + } else { + tmpDir = topTmpDir; + } +- chownToBuilder(tmpDir); ++ ++ /* The TOCTOU between the previous mkdir call and this open call is unavoidable due to ++ POSIX semantics.*/ ++ tmpDirFd = AutoCloseFD{open(tmpDir.c_str(), O_RDONLY | O_NOFOLLOW | O_DIRECTORY)}; ++ if (!tmpDirFd) ++ throw SysError("failed to open the build temporary directory descriptor '%1%'", tmpDir); ++ ++ chownToBuilder(tmpDirFd.get(), tmpDir); + + for (auto & [outputName, status] : initialOutputs) { + /* Set scratch path we'll actually use during the build. +@@ -1469,9 +1496,7 @@ void DerivationBuilderImpl::initTmpDir() + } else { + auto hash = hashString(HashAlgorithm::SHA256, i.first); + std::string fn = ".attr-" + hash.to_string(HashFormat::Nix32, false); +- Path p = tmpDir + "/" + fn; +- writeFile(p, rewriteStrings(i.second, inputRewrites)); +- chownToBuilder(p); ++ writeBuilderFile(fn, rewriteStrings(i.second, inputRewrites)); + env[i.first + "Path"] = tmpDirInSandbox + "/" + fn; + } + } +@@ -1580,11 +1605,9 @@ void DerivationBuilderImpl::writeStructuredAttrs() + + auto jsonSh = StructuredAttrs::writeShell(json); + +- writeFile(tmpDir + "/.attrs.sh", rewriteStrings(jsonSh, inputRewrites)); +- chownToBuilder(tmpDir + "/.attrs.sh"); ++ writeBuilderFile(".attrs.sh", rewriteStrings(jsonSh, inputRewrites)); + env["NIX_ATTRS_SH_FILE"] = tmpDirInSandbox + "/.attrs.sh"; +- writeFile(tmpDir + "/.attrs.json", rewriteStrings(json.dump(), inputRewrites)); +- chownToBuilder(tmpDir + "/.attrs.json"); ++ writeBuilderFile(".attrs.json", rewriteStrings(json.dump(), inputRewrites)); + env["NIX_ATTRS_JSON_FILE"] = tmpDirInSandbox + "/.attrs.json"; + } + } +@@ -1838,6 +1861,24 @@ void setupSeccomp() + #endif + } + ++void DerivationBuilderImpl::chownToBuilder(int fd, const Path & path) ++{ ++ if (!buildUser) return; ++ if (fchown(fd, buildUser->getUID(), buildUser->getGID()) == -1) ++ throw SysError("cannot change ownership of file '%1%'", path); ++} ++ ++void DerivationBuilderImpl::writeBuilderFile( ++ const std::string & name, ++ std::string_view contents) ++{ ++ auto path = std::filesystem::path(tmpDir) / name; ++ AutoCloseFD fd{openat(tmpDirFd.get(), name.c_str(), O_WRONLY | O_TRUNC | O_CREAT | O_CLOEXEC | O_EXCL | O_NOFOLLOW, 0666)}; ++ if (!fd) ++ throw SysError("creating file %s", path); ++ writeFile(fd, path, contents); ++ chownToBuilder(fd.get(), path); ++} + + void DerivationBuilderImpl::runChild() + { +@@ -3043,6 +3084,15 @@ void DerivationBuilderImpl::checkOutputs(const std::mapd_name; + if (childName == "." || childName == "..") continue; +- _deletePath(dirfd(dir.get()), path + "/" + childName, bytesFreed); ++ _deletePath(dirfd(dir.get()), path / childName, bytesFreed); + } + if (errno) throw SysError("reading directory %1%", path); + } +@@ -490,14 +498,13 @@ static void _deletePath(Descriptor parentfd, const std::filesystem::path & path, + + static void _deletePath(const std::filesystem::path & path, uint64_t & bytesFreed) + { +- Path dir = dirOf(path.string()); +- if (dir == "") +- dir = "/"; ++ assert(path.is_absolute()); ++ assert(path.parent_path() != path); + +- AutoCloseFD dirfd = toDescriptor(open(dir.c_str(), O_RDONLY)); ++ AutoCloseFD dirfd = toDescriptor(open(path.parent_path().string().c_str(), O_RDONLY)); + if (!dirfd) { + if (errno == ENOENT) return; +- throw SysError("opening directory '%1%'", path); ++ throw SysError("opening directory %s", path.parent_path()); + } + + _deletePath(dirfd.get(), path, bytesFreed); +diff --git a/src/libutil/include/nix/util/file-system.hh b/src/libutil/include/nix/util/file-system.hh +index b8fa4cfa0..a9a6e43bf 100644 +--- a/src/libutil/include/nix/util/file-system.hh ++++ b/src/libutil/include/nix/util/file-system.hh +@@ -175,21 +175,27 @@ std::string readFile(const Path & path); + std::string readFile(const std::filesystem::path & path); + void readFile(const Path & path, Sink & sink, bool memory_map = true); + ++enum struct FsSync { Yes, No }; ++ + /** + * Write a string to a file. + */ +-void writeFile(const Path & path, std::string_view s, mode_t mode = 0666, bool sync = false); +-static inline void writeFile(const std::filesystem::path & path, std::string_view s, mode_t mode = 0666, bool sync = false) ++void writeFile(const Path & path, std::string_view s, mode_t mode = 0666, FsSync sync = FsSync::No); ++ ++static inline void writeFile(const std::filesystem::path & path, std::string_view s, mode_t mode = 0666, FsSync sync = FsSync::No) + { + return writeFile(path.string(), s, mode, sync); + } + +-void writeFile(const Path & path, Source & source, mode_t mode = 0666, bool sync = false); +-static inline void writeFile(const std::filesystem::path & path, Source & source, mode_t mode = 0666, bool sync = false) ++void writeFile(const Path & path, Source & source, mode_t mode = 0666, FsSync sync = FsSync::No); ++ ++static inline void writeFile(const std::filesystem::path & path, Source & source, mode_t mode = 0666, FsSync sync = FsSync::No) + { + return writeFile(path.string(), source, mode, sync); + } + ++void writeFile(AutoCloseFD & fd, const Path & origPath, std::string_view s, mode_t mode = 0666, FsSync sync = FsSync::No); ++ + /** + * Flush a path's parent directory to disk. + */ +-- +2.44.1 +