fix env-vars beeing written to /tmp

This overall seems like insecure tmp file handling to me. Because other users could replace files in /tmp with a symlink and make the nix-shell override other files. fixes NixOS#11470
Mic92 · Oct 22, 2024 · 842cdf0 · 842cdf0
1 parent 2e0853c
commit 842cdf0
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 12 deletions.
diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc
@@ -526,8 +526,6 @@ static void main_nix_build(int argc, char * * argv)
         // Set the environment.
         auto env = getEnv();
 
-        auto tmp = getEnvNonEmpty("TMPDIR").value_or("/tmp");
-
         if (pure) {
             decltype(env) newEnv;
             for (auto & i : env)
@@ -538,18 +536,16 @@ static void main_nix_build(int argc, char * * argv)
             env["__ETC_PROFILE_SOURCED"] = "1";
         }
 
-        env["NIX_BUILD_TOP"] = env["TMPDIR"] = env["TEMPDIR"] = env["TMP"] = env["TEMP"] = tmp;
+        env["NIX_BUILD_TOP"] = env["TMPDIR"] = env["TEMPDIR"] = env["TMP"] = env["TEMP"] = tmpDir.path();
         env["NIX_STORE"] = store->storeDir;
         env["NIX_BUILD_CORES"] = std::to_string(settings.buildCores);
 
         auto passAsFile = tokenizeString<StringSet>(getOr(drv.env, "passAsFile", ""));
 
-        bool keepTmp = false;
         int fileNr = 0;
 
         for (auto & var : drv.env)
             if (passAsFile.count(var.first)) {
-                keepTmp = true;
                 auto fn = ".attr-" + std::to_string(fileNr++);
                 Path p = (tmpDir.path() / fn).string();
                 writeFile(p, var.second);
@@ -591,7 +587,6 @@ static void main_nix_build(int argc, char * * argv)
 
                 env["NIX_ATTRS_SH_FILE"] = attrsSH;
                 env["NIX_ATTRS_JSON_FILE"] = attrsJSON;
-                keepTmp = true;
             }
         }
 
@@ -601,12 +596,10 @@ static void main_nix_build(int argc, char * * argv)
            lose the current $PATH directories. */
         auto rcfile = (tmpDir.path() / "rc").string();
         std::string rc = fmt(
-                R"(_nix_shell_clean_tmpdir() { command rm -rf %1%; }; )"s +
-                (keepTmp ?
-                    "trap _nix_shell_clean_tmpdir EXIT; "
-                    "exitHooks+=(_nix_shell_clean_tmpdir); "
-                    "failureHooks+=(_nix_shell_clean_tmpdir); ":
-                    "_nix_shell_clean_tmpdir; ") +
+                (R"(_nix_shell_clean_tmpdir() { command rm -rf %1%; };)"s
+                  "trap _nix_shell_clean_tmpdir EXIT; "
+                  "exitHooks+=(_nix_shell_clean_tmpdir); "
+                  "failureHooks+=(_nix_shell_clean_tmpdir); ") +
                 (pure ? "" : "[ -n \"$PS1\" ] && [ -e ~/.bashrc ] && source ~/.bashrc;") +
                 "%2%"
                 // always clear PATH.

diff --git a/tests/functional/nix-shell.sh b/tests/functional/nix-shell.sh
@@ -31,6 +31,15 @@ output=$(nix-shell --pure --keep SELECTED_IMPURE_VAR "$shellDotNix" -A shellDrv
 
 [ "$output" = " - foo - bar - baz" ]
 
+# test NIX_BUILD_TOP
+testTmpDir=$(pwd)/nix-shell
+mkdir -p "$testTmpDir"
+output=$(TMPDIR="$testTmpDir" nix-shell --pure "$shellDotNix" -A shellDrv --run 'echo $NIX_BUILD_TOP')
+[[ "$output" =~ ${testTmpDir}.* ]] || {
+    echo "expected $output =~ ${testTmpDir}.*" >&2
+    exit 1
+}
+
 # Test nix-shell on a .drv
 [[ $(nix-shell --pure $(nix-instantiate "$shellDotNix" -A shellDrv) --run \
     'echo "$IMPURE_VAR - $VAR_FROM_STDENV_SETUP - $VAR_FROM_NIX - $TEST_inNixShell"') = " - foo - bar - false" ]]