libexpr: forbid allowed -> disallowed -> allowed links

this is more of a theoretical problem, but it does allow changing the
behavior of a flake depending on mutable machine state. it's unlikely
that this could be used to reliably do anything bad, but it does lead
to even more non-determinstic evaluation of (notionally) pure flakes.

Change-Id: I5bac7ed045046da08a36c764ab887bc9c7551542

lix/libexpr/eval.cc

@@ -282,7 +282,7 @@ EvalPaths::EvalPaths(
     , errors(errors)
 {
     if (evalSettings.restrictEval || evalSettings.pureEval) {
-        allowedPaths = std::optional(PathSet());
+        allowedPaths = AllowedPath{.allowAllChildren = false};
 
         for (auto & i : searchPath_.elements) {
             auto r = resolveSearchPathPath(i.path);
@@ -362,14 +362,23 @@ EvalState::~EvalState()
 
 void EvalPaths::allowPath(const Path & path)
 {
-    if (allowedPaths)
-        allowedPaths->insert(path);
+    if (!allowedPaths) {
+        return;
+    }
+
+    CanonPath p(path);
+    auto * level = &*allowedPaths;
+    for (const auto & entry : p) {
+        level = &level->children.emplace(std::piecewise_construct, std::tuple(entry), std::tuple())
+                     .first->second;
+    }
+    level->allowAllChildren = true;
 }
 
 void EvalPaths::allowPath(const StorePath & storePath)
 {
     if (allowedPaths)
-        allowedPaths->insert(store->toRealPath(storePath));
+        allowPath(store->toRealPath(storePath));
 }
 
 void EvalPaths::allowAndSetStorePathString(const StorePath & storePath, Value & v)
@@ -387,45 +396,78 @@ CheckedSourcePath EvalPaths::checkSourcePath(const SourcePath & path_)
     if (i != resolvedPaths.end())
         return i->second;
 
-    bool found = false;
-
     /* First canonicalize the path without symlinks, so we make sure an
      * attacker can't append ../../... to a path that would be in allowedPaths
      * and thus leak symlink targets.
      */
-    Path abspath = canonPath(path_.canonical().abs());
+    const CanonPath abspath{path_.canonical().abs()};
 
-    if (abspath.starts_with(corepkgsPrefix)) {
-        return SourcePath(CanonPath(abspath)).unsafeIntoChecked();
+    if (abspath.abs().starts_with(corepkgsPrefix)) {
+        return SourcePath(std::move(abspath)).unsafeIntoChecked();
     }
 
-    for (auto & i : *allowedPaths) {
-        if (isDirOrInDir(abspath, i)) {
-            found = true;
-            break;
-        }
-    }
-
-    if (!found) {
-        auto modeInformation = evalSettings.pureEval
-            ? "in pure eval mode (use '--impure' to override)"
-            : "in restricted mode";
-        throw RestrictedPathError("access to absolute path '%1%' is forbidden %2%", abspath, modeInformation);
-    }
-
-    /* Resolve symlinks. */
+    /* Resolve symlinks. This is mostly restricted copy of canonPath with
+       resolveSymlinks=true, because we need access to intermediat paths. */
     debug("checking access to '%s'", abspath);
-    SourcePath path = CanonPath(canonPath(abspath, true));
 
-    for (auto & i : *allowedPaths) {
-        if (isDirOrInDir(path.canonical().abs(), i)) {
-            auto checked = path.unsafeIntoChecked();
-            resolvedPaths.insert_or_assign(path_.canonical().abs(), checked);
-            return checked;
-        }
+    /* Count the number of times we follow a symlink and stop at some
+       arbitrary (but high) limit to prevent infinite loops. */
+    unsigned int followCount = 0, maxFollow = 1024;
+
+    std::optional<CanonPath> componentsBacking;
+    std::vector<std::string_view> components(abspath.begin(), abspath.end());
+
+retry:
+
+    if (++followCount >= maxFollow) {
+        throw Error("infinite symlink recursion in path '%1%'", path_);
     }
 
-    throw RestrictedPathError("access to canonical path '%1%' is forbidden in restricted mode", path);
+    // TODO: tests for this stuff
+    const auto * level = &*allowedPaths;
+    CheckedSourcePath current = SourcePath(CanonPath::root).unsafeIntoChecked();
+    for (auto ct = components.begin(); ct != components.end(); ct++) {
+        auto & p = *ct;
+        // an empty level means all subpaths are allowed, propagate this forwards
+        // by setting level=nullptr for the subsequent checks. a symlink will set
+        // level to the "VFS" root and restart the check with a the resolved path
+        if (level) {
+            if (level->allowAllChildren) {
+                level = nullptr;
+            } else if (auto it = level->children.find(p); it != level->children.end()) {
+                level = &it->second;
+            } else {
+                goto failed;
+            }
+        }
+        auto next = (current + p).unsafeIntoChecked();
+        auto st = next.maybeLstat();
+        // resolve symlinks, treating nonexistant components like regular directories.
+        // this mirrors canonPath behavior and is necessary for `builtins.pathExists`.
+        if (st && st->type == InputAccessor::tSymlink) {
+            auto target = next.readLink();
+            auto levelResolved = target.starts_with("/")
+                ? CanonPath(target)
+                : CanonPath(current.canonical().abs() + "/" + target);
+            for (ct++; ct != components.end(); ct++) {
+                levelResolved.push(*ct);
+            }
+            components = {levelResolved.begin(), levelResolved.end()};
+            componentsBacking = std::move(levelResolved);
+            followCount += 1;
+            goto retry;
+        }
+        current = std::move(next);
+    }
+
+    resolvedPaths.insert_or_assign(path_.canonical().abs(), current);
+    return current;
+
+failed:
+    auto modeInformation = evalSettings.pureEval
+        ? "in pure eval mode (use '--impure' to override)"
+        : "in restricted mode";
+    throw RestrictedPathError("access to absolute path '%1%' is forbidden %2%", abspath, modeInformation);
 }
 
 

lix/libexpr/eval.hh

@@ -376,13 +376,26 @@ public:
 
     const SearchPath & searchPath() const { return searchPath_; }
 
+private:
+    struct AllowedPath
+    {
+        struct ComponentLess : std::less<>
+        {
+            // we'll only use this for string-likes, it's fine. trust me sis.
+            using is_transparent = void;
+        };
+
+        std::map<std::string, AllowedPath, ComponentLess> children;
+
+        bool allowAllChildren = false;
+    };
+
     /**
      * The allowed filesystem paths in restricted or pure evaluation
      * mode.
      */
-    std::optional<PathSet> allowedPaths;
+    std::optional<AllowedPath> allowedPaths;
 
-private:
 
     /* Cache for calls to addToStore(); maps source paths to the store
        paths. */

lix/libutil/canon-path.hh

@@ -84,6 +84,11 @@ public:
 
     struct Iterator
     {
+        using difference_type = void;
+        using value_type = std::string_view;
+        using reference = std::string_view;
+        using iterator_category = std::input_iterator_tag;
+
         std::string_view remaining;
         size_t slash;
 
@@ -101,7 +106,7 @@ public:
         const std::string_view operator * () const
         { return remaining.substr(0, slash); }
 
-        void operator ++ ()
+        Iterator & operator ++ ()
         {
             if (slash == remaining.npos)
                 remaining = remaining.substr(remaining.size());
@@ -109,6 +114,14 @@ public:
                 remaining = remaining.substr(slash + 1);
                 slash = remaining.find('/');
             }
+            return *this;
+        }
+
+        Iterator operator++(int)
+        {
+            auto result = *this;
+            ++*this;
+            return result;
         }
     };