From ff7b9a1fd3c9c692c09b5f6748cda5aa059f9a4c Mon Sep 17 00:00:00 2001 From: Emily Date: Thu, 4 Jul 2024 16:19:51 +0100 Subject: [PATCH 1/2] libstore: clean up the build directory properly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After the fix for CVE-2024-38531, this was only removing the nested build directory, rather than the top‐level temporary directory. Fixes: 1d3696f0fb88d610abc234a60e0d6d424feafdf1 (cherry picked from commit 76e4adfaac3083056e79b518ccc197a7645a0f2d) (cherry picked from commit 0d68b40dda02f2907be91f7876128011e3b72cad) --- src/libstore/build/local-derivation-goal.cc | 9 +++++---- src/libstore/build/local-derivation-goal.hh | 8 +++++++- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 98ba6af0f..45b44951a 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -484,12 +484,12 @@ void LocalDerivationGoal::startBuilder() /* Create a temporary directory where the build will take place. */ - tmpDir = createTempDir("", "nix-build-" + std::string(drvPath.name()), false, false, 0700); + topTmpDir = createTempDir("", "nix-build-" + std::string(drvPath.name()), false, false, 0700); if (useChroot) { /* If sandboxing is enabled, put the actual TMPDIR underneath an inaccessible root-owned directory, to prevent outside access. */ - tmpDir = tmpDir + "/build"; + tmpDir = topTmpDir + "/build"; createDir(tmpDir, 0700); } chownToBuilder(tmpDir); @@ -2957,7 +2957,7 @@ void LocalDerivationGoal::checkOutputs(const std::mapisBuiltin()) { @@ -2965,7 +2965,8 @@ void LocalDerivationGoal::deleteTmpDir(bool force) chmod(tmpDir.c_str(), 0755); } else - deletePath(tmpDir); + deletePath(topTmpDir); + topTmpDir = ""; tmpDir = ""; } } diff --git a/src/libstore/build/local-derivation-goal.hh b/src/libstore/build/local-derivation-goal.hh index 5fa91ae31..86b86c01e 100644 --- a/src/libstore/build/local-derivation-goal.hh +++ b/src/libstore/build/local-derivation-goal.hh @@ -26,10 +26,16 @@ struct LocalDerivationGoal : public DerivationGoal std::optional cgroup; /** - * The temporary directory. + * The temporary directory used for the build. */ Path tmpDir; + /** + * The top-level temporary directory. `tmpDir` is either equal to + * or a child of this directory. + */ + Path topTmpDir; + /** * The path of the temporary directory in the sandbox. */ From 0b97319ed478658b4907ef1b9f4ef2264cf1b854 Mon Sep 17 00:00:00 2001 From: Emily Date: Thu, 4 Jul 2024 16:24:59 +0100 Subject: [PATCH 2/2] libstore: fix sandboxed builds on macOS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The recent fix for CVE-2024-38531 broke the sandbox on macOS completely. As it’s not practical to use `chroot(2)` on macOS, the build takes place in the main filesystem tree, and the world‐unreadable wrapper directory prevents the build from accessing its `$TMPDIR` at all. The macOS sandbox probably shouldn’t be treated as any kind of a security boundary in its current state, but this specific vulnerability wasn’t possible to exploit on macOS anyway, as creating `set{u,g}id` binaries is blocked by sandbox policy. Locking down the build sandbox further may be a good idea in future, but it already has significant compatibility issues. For now, restore the previous status quo on macOS. Thanks to @alois31 for helping me come to a better understanding of the vulnerability. Fixes: 1d3696f0fb88d610abc234a60e0d6d424feafdf1 Closes: #11002 (cherry picked from commit af2e1142b17dadf24a0b66d8973033dce6efa32b) (cherry picked from commit 9feee13952bdd46069cf22ef3d67b8b1b7f47c39) --- src/libstore/build/local-derivation-goal.cc | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 45b44951a..5ab231f24 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -485,12 +485,22 @@ void LocalDerivationGoal::startBuilder() /* Create a temporary directory where the build will take place. */ topTmpDir = createTempDir("", "nix-build-" + std::string(drvPath.name()), false, false, 0700); +#if __APPLE__ + if (false) { +#else if (useChroot) { +#endif /* If sandboxing is enabled, put the actual TMPDIR underneath an inaccessible root-owned directory, to prevent outside - access. */ + access. + + On macOS, we don't use an actual chroot, so this isn't + possible. Any mitigation along these lines would have to be + done directly in the sandbox profile. */ tmpDir = topTmpDir + "/build"; createDir(tmpDir, 0700); + } else { + tmpDir = topTmpDir; } chownToBuilder(tmpDir);