From: Thomas Markwalder <tmark@isc.org>
Date: Thu, 6 Aug 2026 19:53:53 +0000 (-0400)
Subject: [#4647] Always treat "." and ".." as invalid
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=97ebe932fa451367a33472280e54f990951ea204;p=thirdparty%2Fkea.git

[#4647] Always treat "." and ".." as invalid

/changelog_unreleased/4647-validatepath-allows-lease-file-path-outside-dhcp-data-dir
    - reworded to be more user friendly

/src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc
    updated test

/src/lib/util/filesystem.cc
    PathChecker::validatePath() - treat "." and ".." as always invalid

/src/lib/util/tests/filesystem_unittests.cc
    updated test
---

diff --git a/changelog_unreleased/4647-validatepath-allows-lease-file-path-outside-dhcp-data-dir b/changelog_unreleased/4647-validatepath-allows-lease-file-path-outside-dhcp-data-dir
index 9621a06ca6..bf801211fb 100644
--- a/changelog_unreleased/4647-validatepath-allows-lease-file-path-outside-dhcp-data-dir
+++ b/changelog_unreleased/4647-validatepath-allows-lease-file-path-outside-dhcp-data-dir
@@ -1,6 +1,4 @@
-[bug]		wlodek
-	PathChecker::validatePath() now rejects "." and ".." file names,
-	which previously bypassed parent-directory checks and could resolve
-	paths outside the configured data directory (e.g. memfile lease
-	database name "..").
+[bug]		wlodek, tmark
+	Path validation now rejects now rejects "." and ".." file names
+	as invalid (e.g. memfile lease database name "..").
 	(Gitlab #4647)
diff --git a/src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc b/src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc
index 60fdee0450..b22228a908 100644
--- a/src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc
+++ b/src/lib/dhcpsrv/tests/memfile_lease_mgr_unittest.cc
@@ -567,13 +567,25 @@ TEST_F(MemfileLeaseMgrTest, defaultDataDir) {
     ASSERT_THROW_MSG(lease_mgr.reset(new Memfile_LeaseMgr(pmap)),
                      file::SecurityError, os.str());
 
-    // Bare ".." must not resolve outside the data directory.
+    // Blank value should be rejected.
+    pmap["name"] = "   ";
+    ASSERT_THROW_MSG(lease_mgr.reset(new Memfile_LeaseMgr(pmap)),
+                     BadValue, "path: '' has no filename");
+
+    // Empty value should be rejected.
+    pmap["name"] = "";
+    ASSERT_THROW_MSG(lease_mgr.reset(new Memfile_LeaseMgr(pmap)),
+                     BadValue, "path: '' has no filename");
+
+    // Bare "." should be rejected.
+    pmap["name"] = ".";
+    ASSERT_THROW_MSG(lease_mgr.reset(new Memfile_LeaseMgr(pmap)),
+                     BadValue, "path: '.' has no filename");
+
+    // Bare ".." should be rejected.
     pmap["name"] = "..";
-    std::ostringstream os2;
-    os2 << "invalid path specified: '..', supported path is '"
-        << CfgMgr::instance().getDataDir() << "'";
     ASSERT_THROW_MSG(lease_mgr.reset(new Memfile_LeaseMgr(pmap)),
-                     file::SecurityError, os2.str());
+                     BadValue, "path: '..' has no filename");
 }
 
 /// @brief Verifies that the supported path may be overridden with
diff --git a/src/lib/util/filesystem.cc b/src/lib/util/filesystem.cc
index 07cf7a0537..2b745aca3e 100644
--- a/src/lib/util/filesystem.cc
+++ b/src/lib/util/filesystem.cc
@@ -293,28 +293,10 @@ PathChecker::validatePath(const std::string input_path_str,
                           bool enforce_path /* = PathChecker::shouldEnforceSecurity() */) const {
     Path input_path(trim(input_path_str));
     auto filename = input_path.filename();
-    if (filename.empty()) {
+    if (filename.empty() || (filename == ".") || (filename == "..")) {
         isc_throw(BadValue, "path: '" << input_path.str() << "' has no filename");
     }
 
-    // A bare "." or ".." has an empty parent directory, so the check below would
-    // otherwise accept it and append it to the supported path. For "..", that
-    // resolves outside the supported directory (e.g. /var/lib/kea/..). The same
-    // escape also occurs for "<supported-path>/..", whose parent matches.
-    if ((filename == ".") || (filename == "..")) {
-        std::ostringstream oss;
-        oss << "invalid path specified: '"
-            << filename
-            << "', supported path is '"
-            << path_ << "'";
-
-        if (enforce_path) {
-            isc_throw(SecurityError, oss.str());
-        } else {
-            isc_throw(SecurityWarn, oss.str());
-        }
-    }
-
     auto parent_path = input_path.parentPath();
     auto parent_dir = input_path.parentDirectory();
     if (!parent_dir.empty()) {
diff --git a/src/lib/util/tests/filesystem_unittests.cc b/src/lib/util/tests/filesystem_unittests.cc
index ebe35f039c..01c283fb3b 100644
--- a/src/lib/util/tests/filesystem_unittests.cc
+++ b/src/lib/util/tests/filesystem_unittests.cc
@@ -398,27 +398,24 @@ TEST_F(PathCheckerTest, validatePathEnforcePath) {
         __LINE__,
         "..",
         "",
-        string("invalid path specified: '..', supported path is '" +
-               def_path + "'"),
-        true
+        "path: '..' has no filename",
+        false
     },
     {
         // Bare "." refers to the supported directory itself.
         __LINE__,
         ".",
         "",
-        string("invalid path specified: '.', supported path is '" +
-               def_path + "'"),
-        true
+        "path: '.' has no filename",
+        false
     },
     {
         // Supported path plus ".." also escapes despite matching parent.
         __LINE__,
         def_path + "/..",
         "",
-        string("invalid path specified: '..', supported path is '" +
-               def_path + "'"),
-        true
+        string("path: '" + def_path + "/..' has no filename"),
+        false
     }
     };