Fix access control

src/libfetchers/git.cc

@@ -682,22 +682,10 @@ struct GitInputScheme : InputScheme
 
         auto files = listFiles(repoInfo);
 
+        CanonPath repoDir(repoInfo.url);
+
         PathFilter filter = [&](const Path & p) -> bool {
-            abort();
-#if 0
-            assert(hasPrefix(p, repoInfo.url));
-            std::string file(p, repoInfo.url.size() + 1);
-
-            auto st = lstat(p);
-
-            if (S_ISDIR(st.st_mode)) {
-                auto prefix = file + "/";
-                auto i = files.lower_bound(prefix);
-                return i != files.end() && hasPrefix(*i, prefix);
-            }
-
-            return files.count(file);
-#endif
+            return CanonPath(p).removePrefix(repoDir).isAllowed(files);
         };
 
         auto storePath = store->addToStore(input.getName(), repoInfo.url, FileIngestionMethod::Recursive, htSHA256, filter);

src/libfetchers/input-accessor.cc

@@ -181,18 +181,9 @@ struct FSInputAccessorImpl : FSInputAccessor
             return false;
 
         if (allowedPaths) {
-            #if 0
-            // FIXME: this can be done more efficiently.
-            auto p = (std::string) absPath.substr(root.size());
-            if (p == "") p = "/";
-            while (true) {
-                if (allowedPaths->find(p) != allowedPaths->end())
-                    break;
-                if (p == "/")
-                    return false;
-                p = dirOf(p);
-            }
-            #endif
+            auto p = absPath.removePrefix(root);
+            if (!p.isAllowed(*allowedPaths))
+                return false;
         }
 
         return true;

src/libutil/canon-path.cc

@@ -19,6 +19,13 @@ std::optional<CanonPath> CanonPath::parent() const
     return CanonPath(unchecked_t(), path.substr(0, path.rfind('/')));
 }
 
+void CanonPath::pop()
+{
+    assert(!isRoot());
+    auto slash = path.rfind('/');
+    path.resize(std::max((size_t) 1, slash));
+}
+
 CanonPath CanonPath::resolveSymlinks() const
 {
     return CanonPath(unchecked_t(), canonPath(abs(), true));
@@ -33,6 +40,14 @@ bool CanonPath::isWithin(const CanonPath & parent) const
             && path[parent.path.size()] != '/'));
 }
 
+CanonPath CanonPath::removePrefix(const CanonPath & prefix) const
+{
+    assert(isWithin(prefix));
+    if (prefix.isRoot()) return *this;
+    if (path.size() == prefix.path.size()) return root;
+    return CanonPath(unchecked_t(), path.substr(prefix.path.size()));
+}
+
 void CanonPath::extend(const CanonPath & x)
 {
     if (x.isRoot()) return;
@@ -63,6 +78,27 @@ CanonPath CanonPath::operator + (std::string_view c) const
     return res;
 }
 
+bool CanonPath::isAllowed(const std::set<CanonPath> & allowed) const
+{
+    /* Check if `this` is an exact match or the parent of an
+       allowed path. */
+    auto lb = allowed.lower_bound(*this);
+    if (lb != allowed.end()) {
+        if (lb->isWithin(*this))
+            return true;
+    }
+
+    /* Check if a parent of `this` is allowed. */
+    auto path = *this;
+    while (!path.isRoot()) {
+        path.pop();
+        if (allowed.count(path))
+            return true;
+    }
+
+    return false;
+}
+
 std::ostream & operator << (std::ostream & stream, const CanonPath & path)
 {
     stream << path.abs();

src/libutil/canon-path.hh

@@ -4,6 +4,7 @@
 #include <optional>
 #include <cassert>
 #include <iostream>
+#include <set>
 
 namespace nix {
 
@@ -95,6 +96,9 @@ public:
 
     std::optional<CanonPath> parent() const;
 
+    /* Remove the last component. Panics if this path is the root.  */
+    void pop();
+
     std::optional<std::string_view> dirOf() const
     {
         if (isRoot()) return std::nullopt;
@@ -113,8 +117,24 @@ public:
     bool operator != (const CanonPath & x) const
     { return path != x.path; }
 
+    /* Compare paths lexicographically except that path separators
+       are sorted before any other character. That is, in the sorted order
+       a directory is always followed directly by its children. For
+       instance, 'foo' < 'foo/bar' < 'foo!'. */
     bool operator < (const CanonPath & x) const
-    { return path < x.path; }
+    {
+        auto i = path.begin();
+        auto j = x.path.begin();
+        for ( ; i != path.end() && j != x.path.end(); ++i, ++j) {
+            auto c_i = *i;
+            if (c_i == '/') c_i = 0;
+            auto c_j = *j;
+            if (c_j == '/') c_j = 0;
+            if (c_i < c_j) return true;
+            if (c_i > c_j) return false;
+        }
+        return i == path.end() && j != x.path.end();
+    }
 
     CanonPath resolveSymlinks() const;
 
@@ -122,6 +142,8 @@ public:
        `parent`. */
     bool isWithin(const CanonPath & parent) const;
 
+    CanonPath removePrefix(const CanonPath & prefix) const;
+
     /* Append another path to this one. */
     void extend(const CanonPath & x);
 
@@ -132,6 +154,12 @@ public:
     void push(std::string_view c);
 
     CanonPath operator + (std::string_view c) const;
+
+    /* Check whether access to this path is allowed, which is the case
+       if 1) `this` is within any of the `allowed` paths; or 2) any of
+       the `allowed` paths are within `this`. (The latter condition
+       ensures access to the parents of allowed paths.) */
+    bool isAllowed(const std::set<CanonPath> & allowed) const;
 };
 
 std::ostream & operator << (std::ostream & stream, const CanonPath & path);

src/libutil/tests/canon-path.cc

@@ -38,6 +38,25 @@ namespace nix {
         }
     }
 
+    TEST(CanonPath, pop) {
+        CanonPath p("foo/bar/x");
+        ASSERT_EQ(p.abs(), "/foo/bar/x");
+        p.pop();
+        ASSERT_EQ(p.abs(), "/foo/bar");
+        p.pop();
+        ASSERT_EQ(p.abs(), "/foo");
+        p.pop();
+        ASSERT_EQ(p.abs(), "/");
+    }
+
+    TEST(CanonPath, removePrefix) {
+        CanonPath p1("foo/bar");
+        CanonPath p2("foo/bar/a/b/c");
+        ASSERT_EQ(p2.removePrefix(p1).abs(), "/a/b/c");
+        ASSERT_EQ(p1.removePrefix(p1).abs(), "/");
+        ASSERT_EQ(p1.removePrefix(CanonPath("/")).abs(), "/foo/bar");
+    }
+
     TEST(CanonPath, iter) {
         {
             CanonPath p("a//foo/bar//");
@@ -95,4 +114,39 @@ namespace nix {
             ASSERT_TRUE(CanonPath("/").isWithin(CanonPath("/")));
         }
     }
+
+    TEST(CanonPath, sort) {
+        ASSERT_FALSE(CanonPath("foo") < CanonPath("foo"));
+        ASSERT_TRUE (CanonPath("foo") < CanonPath("foo/bar"));
+        ASSERT_TRUE (CanonPath("foo/bar") < CanonPath("foo!"));
+        ASSERT_FALSE(CanonPath("foo!") < CanonPath("foo"));
+        ASSERT_TRUE (CanonPath("foo") < CanonPath("foo!"));
+    }
+
+    TEST(CanonPath, allowed) {
+        {
+            std::set<CanonPath> allowed {
+                CanonPath("foo/bar"),
+                CanonPath("foo!"),
+                CanonPath("xyzzy"),
+                CanonPath("a/b/c"),
+            };
+
+            ASSERT_TRUE (CanonPath("foo/bar").isAllowed(allowed));
+            ASSERT_TRUE (CanonPath("foo/bar/bla").isAllowed(allowed));
+            ASSERT_TRUE (CanonPath("foo").isAllowed(allowed));
+            ASSERT_FALSE(CanonPath("bar").isAllowed(allowed));
+            ASSERT_FALSE(CanonPath("bar/a").isAllowed(allowed));
+            ASSERT_TRUE (CanonPath("a").isAllowed(allowed));
+            ASSERT_TRUE (CanonPath("a/b").isAllowed(allowed));
+            ASSERT_TRUE (CanonPath("a/b/c").isAllowed(allowed));
+            ASSERT_TRUE (CanonPath("a/b/c/d").isAllowed(allowed));
+            ASSERT_TRUE (CanonPath("a/b/c/d/e").isAllowed(allowed));
+            ASSERT_FALSE(CanonPath("a/b/a").isAllowed(allowed));
+            ASSERT_FALSE(CanonPath("a/b/d").isAllowed(allowed));
+            ASSERT_FALSE(CanonPath("aaa").isAllowed(allowed));
+            ASSERT_FALSE(CanonPath("zzz").isAllowed(allowed));
+            ASSERT_TRUE (CanonPath("/").isAllowed(allowed));
+        }
+    }
 }