From fa88f7152070d4c886b512de00691da709bc7229 Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <edolstra@gmail.com>
Date: Wed, 1 May 2019 20:23:39 +0200
Subject: [PATCH] Validate 'dir=' parameters

We reject any path element starting with a '.' (mostly to reject '.'
and '..').
---
 src/libexpr/primops/flakeref.cc | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/libexpr/primops/flakeref.cc b/src/libexpr/primops/flakeref.cc
index 66eab4db6..56ba58d09 100644
--- a/src/libexpr/primops/flakeref.cc
+++ b/src/libexpr/primops/flakeref.cc
@@ -34,6 +34,11 @@ const static std::string pathRegex = "/?" + segmentRegex + "(?:/" + segmentRegex
 // Note: '/' is not a valid query parameter, but so what...
 const static std::string paramRegex = "[a-z]+=[/a-zA-Z0-9._-]*";
 
+// 'dir' path elements cannot start with a '.'. We also reject
+// potentially dangerous characters like ';'.
+const static std::string subDirElemRegex = "(?:[a-zA-Z0-9_-]+[a-zA-Z0-9._-]*)";
+const static std::string subDirRegex = subDirElemRegex + "(?:/" + subDirElemRegex + ")*";
+
 FlakeRef::FlakeRef(const std::string & uri, bool allowRelative)
 {
     // FIXME: could combine this into one regex.
@@ -55,6 +60,8 @@ FlakeRef::FlakeRef(const std::string & uri, bool allowRelative)
 
     static std::regex refRegex2(refRegex, std::regex::ECMAScript);
 
+    static std::regex subDirRegex2(subDirRegex, std::regex::ECMAScript);
+
     std::cmatch match;
     if (std::regex_match(uri.c_str(), match, flakeRegex)) {
         IsAlias d;
@@ -100,7 +107,8 @@ FlakeRef::FlakeRef(const std::string & uri, bool allowRelative)
                     throw Error("invalid Git ref '%s'", value);
                 ref = value;
             } else if (name == "dir") {
-                // FIXME: validate value; should not contain relative paths
+                if (!std::regex_match(value, subDirRegex2))
+                    throw Error("flake '%s' has invalid subdirectory '%s'", uri, value);
                 subdir = value;
             } else
                 // FIXME: should probably pass through unknown parameters
@@ -124,6 +132,7 @@ FlakeRef::FlakeRef(const std::string & uri, bool allowRelative)
 std::string FlakeRef::to_string() const
 {
     std::string string;
+
     if (auto refData = std::get_if<FlakeRef::IsAlias>(&data))
         string = refData->alias;
 
@@ -142,9 +151,12 @@ std::string FlakeRef::to_string() const
 
     else abort();
 
+    // FIXME: need to use ?rev etc. for IsGit URIs.
     string += (ref ? "/" + *ref : "") +
               (rev ? "/" + rev->to_string(Base16, false) : "");
+
     if (subdir != "") string += "?dir=" + subdir;
+
     return string;
 }