From dc89dfa7b3f8fa7368cd4c5e20891e60e418722b Mon Sep 17 00:00:00 2001
From: regnat <rg@regnat.ovh>
Date: Thu, 23 Dec 2021 10:35:09 +0100
Subject: [PATCH] Properly return false on `builtins.pathExists
 /someNonAllowedPath`

Follow-up from https://github.com/NixOS/nix/pull/5807 to fix https://github.com/NixOS/nix/pull/5807#issuecomment-1000135394
---
 src/libexpr/primops.cc | 29 ++++++++++++++++++++++-------
 tests/pure-eval.sh     |  3 +++
 2 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
index aff8f951e..62c21c7c5 100644
--- a/src/libexpr/primops.cc
+++ b/src/libexpr/primops.cc
@@ -89,18 +89,28 @@ StringMap EvalState::realiseContext(const PathSet & context)
     return res;
 }
 
-static Path realisePath(EvalState & state, const Pos & pos, Value & v, bool requireAbsolutePath = true)
+struct RealisePathFlags {
+    // Whether to check whether the path is a valid absolute path
+    bool requireAbsolutePath = true;
+    // Whether to check that the path is allowed in pure eval mode
+    bool checkForPureEval = true;
+};
+
+static Path realisePath(EvalState & state, const Pos & pos, Value & v, const RealisePathFlags flags = {})
 {
     PathSet context;
 
-    Path path = requireAbsolutePath
+    Path path = flags.requireAbsolutePath
         ? state.coerceToPath(pos, v, context)
         : state.coerceToString(pos, v, context, false, false);
 
     StringMap rewrites = state.realiseContext(context);
 
-    return state.checkSourcePath(
-            state.toRealPath(rewriteStrings(path, rewrites), context));
+    auto realPath = state.toRealPath(rewriteStrings(path, rewrites), context);
+
+    return flags.checkForPureEval
+        ? state.checkSourcePath(realPath)
+        : realPath;
 }
 
 /* Add and attribute to the given attribute map from the output name to
@@ -1371,7 +1381,12 @@ static void prim_pathExists(EvalState & state, const Pos & pos, Value * * args,
 {
     Path path;
     try {
-        path = realisePath(state, pos, *args[0]);
+        // We don’t check the path right now, because we don’t want to throw if
+        // the path isn’t allowed, but just return false
+        // (and we can’t just catch the exception here because we still want to
+        // throw if something in the evaluation of `*args[0]` tries to access an
+        // unauthorized path)
+        path = realisePath(state, pos, *args[0], { .checkForPureEval = false });
     } catch (InvalidPathError & e) {
         throw EvalError({
             .msg = hintfmt(
@@ -1382,7 +1397,7 @@ static void prim_pathExists(EvalState & state, const Pos & pos, Value * * args,
     }
 
     try {
-        mkBool(v, pathExists(path));
+        mkBool(v, pathExists(state.checkSourcePath(path)));
     } catch (SysError & e) {
         /* Don't give away info from errors while canonicalising
            ‘path’ in restricted mode. */
@@ -1496,7 +1511,7 @@ static void prim_findFile(EvalState & state, const Pos & pos, Value * * args, Va
         Path path;
 
         try {
-            path = realisePath(state, pos, *i->value, false);
+            path = realisePath(state, pos, *i->value, { .requireAbsolutePath = false });
         } catch (InvalidPathError & e) {
             throw EvalError({
                 .msg = hintfmt("cannot find '%1%', since path '%2%' is not valid", path, e.path),
diff --git a/tests/pure-eval.sh b/tests/pure-eval.sh
index cb4b5c5fc..1a4568ea6 100644
--- a/tests/pure-eval.sh
+++ b/tests/pure-eval.sh
@@ -11,6 +11,9 @@ missingImpureErrorMsg=$(! nix eval --expr 'builtins.readFile ./pure-eval.sh' 2>&
 echo "$missingImpureErrorMsg" | grep -q -- --impure || \
     fail "The error message should mention the “--impure” flag to unblock users"
 
+[[ $(nix eval --expr 'builtins.pathExists ./pure-eval.sh') == false ]] || \
+    fail "Calling 'pathExists' on a non-authorised path should return false"
+
 (! nix eval --expr builtins.currentTime)
 (! nix eval --expr builtins.currentSystem)