local-binary-cache-store: Check that paths don't escape the binary cache directory

Previously arguments to getFile, upsertFile weren't checked
to be inside the root directory. It's not a very big issue since
substituters/stores are already a trusted component and can't be
specified without being a trusted user. Still, it's nice to validate
the necessary preconditions. Also changes the binaryCacheDir to be a
std::filesystem::path. Note the gotcha with absolute paths and operator/.

src/libstore/include/nix/store/local-binary-cache-store.hh

@@ -14,7 +14,7 @@ struct LocalBinaryCacheStoreConfig : std::enable_shared_from_this<LocalBinaryCac
      */
     LocalBinaryCacheStoreConfig(std::string_view scheme, PathView binaryCacheDir, const Params & params);
 
-    Path binaryCacheDir;
+    std::filesystem::path binaryCacheDir;
 
     static const std::string name()
     {

src/libstore/local-binary-cache-store.cc

@@ -3,11 +3,32 @@
 #include "nix/store/nar-info-disk-cache.hh"
 #include "nix/util/signals.hh"
 #include "nix/store/store-registration.hh"
+#include "nix/util/url.hh"
 
 #include <atomic>
 
 namespace nix {
 
+static std::filesystem::path checkBinaryCachePath(const std::filesystem::path & root, std::string_view path)
+{
+    /* Note: these checks aren't complete and don't guard against symlink shenanigans. */
+    auto p = std::filesystem::path(path);
+    if (p.is_absolute())
+        /* Never happens unless the caller is messed up. */
+        throw Error("binary cache path '%s' must be relative", path);
+
+    auto result = (root / p).lexically_normal();
+
+    /* NB: lexically_normal() only does textual normalization and does
+       not resolve symlinks. This is acceptable because store/substituter
+       paths are already trusted, and this check is defense-in-depth
+       against ".." traversal. */
+    if (!isInDir(result, root.lexically_normal()))
+        throw Error("binary cache path '%s' escapes cache directory", path);
+
+    return result;
+}
+
 LocalBinaryCacheStoreConfig::LocalBinaryCacheStoreConfig(
     std::string_view scheme, PathView binaryCacheDir, const StoreReference::Params & params)
     : Store::Config{params}
@@ -29,7 +50,7 @@ StoreReference LocalBinaryCacheStoreConfig::getReference() const
         .variant =
             StoreReference::Specified{
                 .scheme = "file",
-                .authority = binaryCacheDir,
+                .authority = encodeUrlPath(pathToUrlPath(binaryCacheDir)),
             },
     };
 }
@@ -56,7 +77,9 @@ protected:
     void upsertFile(
         const std::string & path, RestartableSource & source, const std::string & mimeType, uint64_t sizeHint) override
     {
-        auto path2 = std::filesystem::path{config->binaryCacheDir} / path;
+        /* TODO: Maybe use RestoreSink for writing stuff? It would have to gain the ability to write files
+           atomically (maybe with O_TMPFILE + linkat + AT_EMPTY_PATH when available or fallback to rename). */
+        auto path2 = checkBinaryCachePath(config->binaryCacheDir, path);
         static std::atomic<int> counter{0};
         createDirs(path2.parent_path());
         auto tmp = path2;
@@ -70,7 +93,7 @@ protected:
     void getFile(const std::string & path, Sink & sink) override
     {
         try {
-            readFile(config->binaryCacheDir + "/" + path, sink);
+            readFile(checkBinaryCachePath(config->binaryCacheDir, path), sink);
         } catch (SystemError & e) {
             if (e.is(std::errc::no_such_file_or_directory))
                 throw NoSuchBinaryCacheFile("file '%s' does not exist in binary cache", path);
@@ -101,17 +124,17 @@ protected:
 
 void LocalBinaryCacheStore::init()
 {
-    createDirs(config->binaryCacheDir + "/nar");
-    createDirs(config->binaryCacheDir + "/" + realisationsPrefix);
+    createDirs(config->binaryCacheDir / "nar");
+    createDirs(config->binaryCacheDir / realisationsPrefix);
     if (config->writeDebugInfo)
-        createDirs(config->binaryCacheDir + "/debuginfo");
-    createDirs(config->binaryCacheDir + "/log");
+        createDirs(config->binaryCacheDir / "debuginfo");
+    createDirs(config->binaryCacheDir / "log");
     BinaryCacheStore::init();
 }
 
 bool LocalBinaryCacheStore::fileExists(const std::string & path)
 {
-    return pathExists(config->binaryCacheDir + "/" + path);
+    return pathExists(checkBinaryCachePath(config->binaryCacheDir, path));
 }
 
 StringSet LocalBinaryCacheStoreConfig::uriSchemes()

src/libutil/file-system.cc

@@ -21,7 +21,9 @@
 #include <sys/time.h>
 #include <unistd.h>
 
+#include <boost/exception/diagnostic_information.hpp>
 #include <boost/iostreams/device/mapped_file.hpp>
+#include <boost/filesystem/path.hpp>
 
 #ifdef __FreeBSD__
 #  include <sys/param.h>
@@ -305,25 +307,27 @@ std::string readFile(const std::filesystem::path & path)
     return readFile(fd.get());
 }
 
-void readFile(const Path & path, Sink & sink, bool memory_map)
+void readFile(const std::filesystem::path & path, Sink & sink, bool memory_map)
 {
     // Memory-map the file for faster processing where possible.
     if (memory_map) {
         try {
-            boost::iostreams::mapped_file_source mmap(path);
+            // mapped_file_source can't be constructed from std::filesystem::path with wide paths. Go
+            // through boost::filesystem::path.
+            boost::iostreams::mapped_file_source mmap(boost::filesystem::path{path.native()});
             if (mmap.is_open()) {
                 sink({mmap.data(), mmap.size()});
                 return;
             }
         } catch (const boost::exception & e) {
+            debug("memory-mapping failed for path: %s: %s", PathFmt(path), boost::diagnostic_information(e));
         }
-        debug("memory-mapping failed for path: %s", path);
     }
 
     // Stream the file instead if memory-mapping fails or is disabled.
-    AutoCloseFD fd = openFileReadonly(std::filesystem::path(path));
+    AutoCloseFD fd = openFileReadonly(path);
     if (!fd)
-        throw NativeSysError("opening file %s", path);
+        throw NativeSysError("opening file %s", PathFmt(path));
     drainFD(fd.get(), sink);
 }
 

src/libutil/include/nix/util/file-system.hh

@@ -244,7 +244,7 @@ Descriptor openNewFileForWrite(const std::filesystem::path & path, mode_t mode,
  */
 std::string readFile(const Path & path);
 std::string readFile(const std::filesystem::path & path);
-void readFile(const Path & path, Sink & sink, bool memory_map = true);
+void readFile(const std::filesystem::path & path, Sink & sink, bool memory_map = true);
 
 enum struct FsSync { Yes, No };