Merge remote-tracking branch 'nix-ghsa-wf4c-57rh-9pjg/advisory-fix-1-2.23' into 2.23-maintenance

configure.ac

@@ -62,12 +62,16 @@ AC_CHECK_TOOL([AR], [ar])
 AC_SYS_LARGEFILE
 
 
-# Solaris-specific stuff.
+# OS-specific stuff.
 case "$host_os" in
   solaris*)
     # Solaris requires -lsocket -lnsl for network functions
     LDFLAGS="-lsocket -lnsl $LDFLAGS"
     ;;
+  darwin*)
+    # Need to link to libsandbox.
+    LDFLAGS="-lsandbox $LDFLAGS"
+    ;;
 esac
 
 

package.nix

@@ -27,6 +27,7 @@
 , libseccomp
 , libsodium
 , man
+, darwin
 , lowdown
 , mdbook
 , mdbook-linkcheck
@@ -250,6 +251,7 @@ in {
     gtest
     rapidcheck
   ] ++ lib.optional stdenv.isLinux libseccomp
+    ++ lib.optional stdenv.hostPlatform.isDarwin darwin.apple_sdk.libs.sandbox
     ++ lib.optional stdenv.hostPlatform.isx86_64 libcpuid
     # There have been issues building these dependencies
     ++ lib.optional (stdenv.hostPlatform == stdenv.buildPlatform && (stdenv.isLinux || stdenv.isDarwin))

src/libstore/unix/build/local-derivation-goal.cc

@@ -58,6 +58,10 @@
 #if __APPLE__
 #include <spawn.h>
 #include <sys/sysctl.h>
+#include <sandbox.h>
+
+/* This definition is undocumented but depended upon by all major browsers. */
+extern "C" int sandbox_init_with_parameters(const char *profile, uint64_t flags, const char *const parameters[], char **errorbuf);
 #endif
 
 #include <pwd.h>
@@ -2029,154 +2033,130 @@ void LocalDerivationGoal::runChild()
                 throw SysError("setuid failed");
         }
 
-        /* Fill in the arguments. */
-        Strings args;
-
-        std::string builder = "invalid";
-
-        if (drv->isBuiltin()) {
-            ;
-        }
 #if __APPLE__
-        else {
+        /* This has to appear before import statements. */
-            /* This has to appear before import statements. */
+        std::string sandboxProfile = "(version 1)\n";
-            std::string sandboxProfile = "(version 1)\n";
 
-            if (useChroot) {
+        if (useChroot) {
 
-                /* Lots and lots and lots of file functions freak out if they can't stat their full ancestry */
+            /* Lots and lots and lots of file functions freak out if they can't stat their full ancestry */
-                PathSet ancestry;
+            PathSet ancestry;
 
-                /* We build the ancestry before adding all inputPaths to the store because we know they'll
+            /* We build the ancestry before adding all inputPaths to the store because we know they'll
-                   all have the same parents (the store), and there might be lots of inputs. This isn't
+               all have the same parents (the store), and there might be lots of inputs. This isn't
-                   particularly efficient... I doubt it'll be a bottleneck in practice */
+               particularly efficient... I doubt it'll be a bottleneck in practice */
-                for (auto & i : pathsInChroot) {
+            for (auto & i : pathsInChroot) {
-                    Path cur = i.first;
+                Path cur = i.first;
-                    while (cur.compare("/") != 0) {
-                        cur = dirOf(cur);
-                        ancestry.insert(cur);
-                    }
-                }
-
-                /* And we want the store in there regardless of how empty pathsInChroot. We include the innermost
-                   path component this time, since it's typically /nix/store and we care about that. */
-                Path cur = worker.store.storeDir;
                 while (cur.compare("/") != 0) {
-                    ancestry.insert(cur);
                     cur = dirOf(cur);
+                    ancestry.insert(cur);
                 }
+            }
 
-                /* Add all our input paths to the chroot */
+            /* And we want the store in there regardless of how empty pathsInChroot. We include the innermost
-                for (auto & i : inputPaths) {
+               path component this time, since it's typically /nix/store and we care about that. */
-                    auto p = worker.store.printStorePath(i);
+            Path cur = worker.store.storeDir;
-                    pathsInChroot[p] = p;
+            while (cur.compare("/") != 0) {
-                }
+                ancestry.insert(cur);
+                cur = dirOf(cur);
+            }
 
-                /* Violations will go to the syslog if you set this. Unfortunately the destination does not appear to be configurable */
+            /* Add all our input paths to the chroot */
-                if (settings.darwinLogSandboxViolations) {
+            for (auto & i : inputPaths) {
-                    sandboxProfile += "(deny default)\n";
+                auto p = worker.store.printStorePath(i);
-                } else {
+                pathsInChroot[p] = p;
-                    sandboxProfile += "(deny default (with no-log))\n";
+            }
-                }
 
-                sandboxProfile +=
+            /* Violations will go to the syslog if you set this. Unfortunately the destination does not appear to be configurable */
-                    #include "sandbox-defaults.sb"
+            if (settings.darwinLogSandboxViolations) {
-                    ;
+                sandboxProfile += "(deny default)\n";
-
-                if (!derivationType->isSandboxed())
-                    sandboxProfile +=
-                        #include "sandbox-network.sb"
-                        ;
-
-                /* Add the output paths we'll use at build-time to the chroot */
-                sandboxProfile += "(allow file-read* file-write* process-exec\n";
-                for (auto & [_, path] : scratchOutputs)
-                    sandboxProfile += fmt("\t(subpath \"%s\")\n", worker.store.printStorePath(path));
-
-                sandboxProfile += ")\n";
-
-                /* Our inputs (transitive dependencies and any impurities computed above)
-
-                   without file-write* allowed, access() incorrectly returns EPERM
-                 */
-                sandboxProfile += "(allow file-read* file-write* process-exec\n";
-                for (auto & i : pathsInChroot) {
-                    if (i.first != i.second.source)
-                        throw Error(
-                            "can't map '%1%' to '%2%': mismatched impure paths not supported on Darwin",
-                            i.first, i.second.source);
-
-                    std::string path = i.first;
-                    auto optSt = maybeLstat(path.c_str());
-                    if (!optSt) {
-                        if (i.second.optional)
-                            continue;
-                        throw SysError("getting attributes of required path '%s", path);
-                    }
-                    if (S_ISDIR(optSt->st_mode))
-                        sandboxProfile += fmt("\t(subpath \"%s\")\n", path);
-                    else
-                        sandboxProfile += fmt("\t(literal \"%s\")\n", path);
-                }
-                sandboxProfile += ")\n";
-
-                /* Allow file-read* on full directory hierarchy to self. Allows realpath() */
-                sandboxProfile += "(allow file-read*\n";
-                for (auto & i : ancestry) {
-                    sandboxProfile += fmt("\t(literal \"%s\")\n", i);
-                }
-                sandboxProfile += ")\n";
-
-                sandboxProfile += additionalSandboxProfile;
-            } else
-                sandboxProfile +=
-                    #include "sandbox-minimal.sb"
-                    ;
-
-            debug("Generated sandbox profile:");
-            debug(sandboxProfile);
-
-            Path sandboxFile = tmpDir + "/.sandbox.sb";
-
-            writeFile(sandboxFile, sandboxProfile);
-
-            bool allowLocalNetworking = parsedDrv->getBoolAttr("__darwinAllowLocalNetworking");
-
-            /* The tmpDir in scope points at the temporary build directory for our derivation. Some packages try different mechanisms
-               to find temporary directories, so we want to open up a broader place for them to put their files, if needed. */
-            Path globalTmpDir = canonPath(defaultTempDir(), true);
-
-            /* They don't like trailing slashes on subpath directives */
-            while (!globalTmpDir.empty() && globalTmpDir.back() == '/')
-                globalTmpDir.pop_back();
-
-            if (getEnv("_NIX_TEST_NO_SANDBOX") != "1") {
-                builder = "/usr/bin/sandbox-exec";
-                args.push_back("sandbox-exec");
-                args.push_back("-f");
-                args.push_back(sandboxFile);
-                args.push_back("-D");
-                args.push_back("_GLOBAL_TMP_DIR=" + globalTmpDir);
-                if (allowLocalNetworking) {
-                    args.push_back("-D");
-                    args.push_back(std::string("_ALLOW_LOCAL_NETWORKING=1"));
-                }
-                args.push_back(drv->builder);
             } else {
-                builder = drv->builder;
+                sandboxProfile += "(deny default (with no-log))\n";
-                args.push_back(std::string(baseNameOf(drv->builder)));
+            }
+
+            sandboxProfile +=
+                #include "sandbox-defaults.sb"
+                ;
+
+            if (!derivationType->isSandboxed())
+                sandboxProfile +=
+                    #include "sandbox-network.sb"
+                    ;
+
+            /* Add the output paths we'll use at build-time to the chroot */
+            sandboxProfile += "(allow file-read* file-write* process-exec\n";
+            for (auto & [_, path] : scratchOutputs)
+                sandboxProfile += fmt("\t(subpath \"%s\")\n", worker.store.printStorePath(path));
+
+            sandboxProfile += ")\n";
+
+            /* Our inputs (transitive dependencies and any impurities computed above)
+
+               without file-write* allowed, access() incorrectly returns EPERM
+             */
+            sandboxProfile += "(allow file-read* file-write* process-exec\n";
+            for (auto & i : pathsInChroot) {
+                if (i.first != i.second.source)
+                    throw Error(
+                        "can't map '%1%' to '%2%': mismatched impure paths not supported on Darwin",
+                        i.first, i.second.source);
+
+                std::string path = i.first;
+                auto optSt = maybeLstat(path.c_str());
+                if (!optSt) {
+                    if (i.second.optional)
+                        continue;
+                    throw SysError("getting attributes of required path '%s", path);
+                }
+                if (S_ISDIR(optSt->st_mode))
+                    sandboxProfile += fmt("\t(subpath \"%s\")\n", path);
+                else
+                    sandboxProfile += fmt("\t(literal \"%s\")\n", path);
+            }
+            sandboxProfile += ")\n";
+
+            /* Allow file-read* on full directory hierarchy to self. Allows realpath() */
+            sandboxProfile += "(allow file-read*\n";
+            for (auto & i : ancestry) {
+                sandboxProfile += fmt("\t(literal \"%s\")\n", i);
+            }
+            sandboxProfile += ")\n";
+
+            sandboxProfile += additionalSandboxProfile;
+        } else
+            sandboxProfile +=
+                #include "sandbox-minimal.sb"
+                ;
+
+        debug("Generated sandbox profile:");
+        debug(sandboxProfile);
+
+        bool allowLocalNetworking = parsedDrv->getBoolAttr("__darwinAllowLocalNetworking");
+
+        /* The tmpDir in scope points at the temporary build directory for our derivation. Some packages try different mechanisms
+            to find temporary directories, so we want to open up a broader place for them to put their files, if needed. */
+        Path globalTmpDir = canonPath(defaultTempDir(), true);
+
+        /* They don't like trailing slashes on subpath directives */
+        while (!globalTmpDir.empty() && globalTmpDir.back() == '/')
+            globalTmpDir.pop_back();
+
+        if (getEnv("_NIX_TEST_NO_SANDBOX") != "1") {
+            Strings sandboxArgs;
+            sandboxArgs.push_back("_GLOBAL_TMP_DIR");
+            sandboxArgs.push_back(globalTmpDir);
+            if (allowLocalNetworking) {
+                sandboxArgs.push_back("_ALLOW_LOCAL_NETWORKING");
+                sandboxArgs.push_back("1");
+            }
+            char * sandbox_errbuf = nullptr;
+            if (sandbox_init_with_parameters(sandboxProfile.c_str(), 0, stringsToCharPtrs(sandboxArgs).data(), &sandbox_errbuf)) {
+                writeFull(STDERR_FILENO, fmt("failed to configure sandbox: %s\n", sandbox_errbuf ? sandbox_errbuf : "(null)"));
+                _exit(1);
             }
         }
-#else
-        else {
-            builder = drv->builder;
-            args.push_back(std::string(baseNameOf(drv->builder)));
-        }
 #endif
 
-        for (auto & i : drv->args)
-            args.push_back(rewriteStrings(i, inputRewrites));
-
         /* Indicate that we managed to set up the build environment. */
         writeFull(STDERR_FILENO, std::string("\2\n"));
 
@@ -2207,6 +2187,14 @@ void LocalDerivationGoal::runChild()
             }
         }
 
+        // Now builder is not builtin
+
+        Strings args;
+        args.push_back(std::string(baseNameOf(drv->builder)));
+
+        for (auto & i : drv->args)
+            args.push_back(rewriteStrings(i, inputRewrites));
+
 #if __APPLE__
         posix_spawnattr_t attrp;
 
@@ -2228,9 +2216,9 @@ void LocalDerivationGoal::runChild()
             posix_spawnattr_setbinpref_np(&attrp, 1, &cpu, NULL);
         }
 
-        posix_spawn(NULL, builder.c_str(), NULL, &attrp, stringsToCharPtrs(args).data(), stringsToCharPtrs(envStrs).data());
+        posix_spawn(NULL, drv->builder.c_str(), NULL, &attrp, stringsToCharPtrs(args).data(), stringsToCharPtrs(envStrs).data());
 #else
-        execve(builder.c_str(), stringsToCharPtrs(args).data(), stringsToCharPtrs(envStrs).data());
+        execve(drv->builder.c_str(), stringsToCharPtrs(args).data(), stringsToCharPtrs(envStrs).data());
 #endif
 
         throw SysError("executing '%1%'", drv->builder);