[#3830] Hook libraries must load from default hook dir

/src/lib/util/filesystem.*
    FileManager::validatePath() - new class and function

/src/lib/hooks/hooks_parser.*
    HooksLibrariesParser::validatePath() - new wrapper around FileManager::validatePath()
    HooksLibrariesParser::parse() - now uses validatePath()

/src/lib/hooks/tests/hooks_manager_unittest.cc
    TEST(HooksParser, validatePathEnforcePath)
    TEST(HooksParser, validatePathEnforcePathFalse) - new tests

/src/lib/util/tests/filesystem_unittests.cc
    TEST(FileManager, validatePathEnforcePath)
    TEST(FileManager, validatePathEnforcePathFalse) - new tests

src/lib/hooks/hooks_parser.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2017-2024 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2017-2025 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -10,13 +10,16 @@
 #include <cc/dhcp_config_error.h>
 #include <hooks/hooks_parser.h>
 #include <boost/algorithm/string.hpp>
+#include <util/filesystem.h>
 #include <util/str.h>
+
 #include <vector>
 
 using namespace std;
 using namespace isc::data;
 using namespace isc::hooks;
 using namespace isc::dhcp;
+using namespace isc::util::file;
 
 namespace isc {
 namespace hooks {
@@ -66,23 +69,7 @@ HooksLibrariesParser::parse(HooksConfig& libraries, ConstElementPtr value) {
 
                 // Get the name of the library and add it to the list after
                 // removing quotes.
-                libname = (entry_item.second)->stringValue();
+                libname = validatePath((entry_item.second)->stringValue());
-
-                // Remove leading/trailing quotes and any leading/trailing
-                // spaces.
-                boost::erase_all(libname, "\"");
-                libname = isc::util::str::trim(libname);
-                if (libname.empty()) {
-                    isc_throw(DhcpConfigError, "hooks library configuration"
-                        " error: value of 'library' element must not be"
-                        " blank (" <<
-                        entry_item.second->getPosition() << ")");
-                }
-
-                // If only the name of the library was provided, add the full path.
-                if (libname.find("/") == string::npos) {
-                    libname = HooksLibrariesParser::default_hooks_path_ + "/" + libname;
-                }
 
                 // Note we have found the library name.
                 lib_found = true;
@@ -112,5 +99,12 @@ HooksLibrariesParser::parse(HooksConfig& libraries, ConstElementPtr value) {
     }
 }
 
+std::string
+HooksLibrariesParser::validatePath(const std::string libpath,
+                                   bool enforce_path /* = true */) {
+    return (FileManager::validatePath(HooksLibrariesParser::default_hooks_path_,
+                                      libpath, enforce_path));
+}
+
 }
 }

src/lib/hooks/hooks_parser.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2017-2024 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2017-2025 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -61,6 +61,15 @@ public:
     /// @brief The default installation path for hook libraries, used to generate
     /// full path if only the hook library binary name is provided.
     static std::string default_hooks_path_;
+
+    /// @brief Validates a library path against the supported path for hooks libraries.
+    ///
+    /// @param libpath library path to validate.
+    /// @param enforce_path enables validation against the supported path.  If false
+    /// verifies only that the path contains a file name.
+    ///
+    /// @return validated path
+    static std::string validatePath(const std::string libpath, bool enforce_path = true);
 };
 
 }  // namespace isc::hooks

src/lib/hooks/tests/hooks_manager_unittest.cc

@@ -1,4 +1,4 @@
-// Copyright (C) 2013-2021 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2025 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -8,7 +8,9 @@
 
 #include <hooks/callout_handle.h>
 #include <hooks/hooks_manager.h>
+#include <hooks/hooks_parser.h>
 #include <hooks/server_hooks.h>
+#include <testutils/gtest_utils.h>
 
 #include <hooks/tests/common_test_class.h>
 #define TEST_ASYNC_CALLOUT
@@ -1077,5 +1079,118 @@ TEST_F(HooksManagerTest, UnloadBeforeUnpark) {
     EXPECT_FALSE(unparked);
 }
 
+TEST(HooksParser, validatePathEnforcePath) {
+    std::string def_path(HooksLibrariesParser::default_hooks_path_);
+    struct Scenario {
+        int line_;
+        std::string lib_path_;
+        std::string exp_path_;
+        std::string exp_error_;
+    };
+
+    std::list<Scenario> scenarios = {
+    {
+        // Invalid parent path.
+        __LINE__,
+        "/var/lib/bs/mylib.a",
+        "",
+        string("invalid path specified: '/var/lib/bs/', supported path is '" + def_path + "'")
+    },
+    {
+        // No file name.
+        __LINE__,
+        def_path + "/",
+        "",
+        string ("path: '" + def_path + "/' has no filename")
+    },
+    {
+        // File name only is valid.
+        __LINE__,
+        "mylib.a",
+        def_path + "/mylib.a",
+        ""
+    },
+    {
+        // Valid full path.
+        __LINE__,
+        def_path + "/mylib.a",
+        def_path + "/mylib.a",
+        ""
+    }
+    };
+
+    for (auto scenario : scenarios) {
+        std::ostringstream oss;
+        oss << " Scenario at line: " << scenario.line_;
+        SCOPED_TRACE(oss.str());
+        std::string validated_path;
+        if (scenario.exp_error_.empty()) {
+            ASSERT_NO_THROW_LOG(validated_path =
+                                HooksLibrariesParser::validatePath(scenario.lib_path_));
+            EXPECT_EQ(validated_path, scenario.exp_path_);
+        } else {
+            ASSERT_THROW_MSG(validated_path =
+                             HooksLibrariesParser::validatePath(scenario.lib_path_),
+                             BadValue, scenario.exp_error_);
+        }
+    }
+}
+
+TEST(HooksParser, validatePathEnforcePathFalse) {
+    std::string def_path(HooksLibrariesParser::default_hooks_path_);
+    struct Scenario {
+        int line_;
+        std::string lib_path_;
+        std::string exp_path_;
+        std::string exp_error_;
+    };
+
+    std::list<Scenario> scenarios = {
+    {
+        // Invalid parent path will fly.
+        __LINE__,
+        "/var/lib/bs/mylib.a",
+        "/var/lib/bs/mylib.a",
+        "",
+    },
+    {
+        // No file name.
+        __LINE__,
+        def_path + "/",
+        "",
+        string ("path: '" + def_path + "/' has no filename")
+    },
+    {
+        // File name only is valid.
+        __LINE__,
+        "mylib.a",
+        def_path + "/mylib.a",
+        ""
+    },
+    {
+        // Valid full path.
+        __LINE__,
+        def_path + "/mylib.a",
+        def_path + "/mylib.a",
+        ""
+    }
+    };
+
+    for (auto scenario : scenarios) {
+        std::ostringstream oss;
+        oss << " Scenario at line: " << scenario.line_;
+        SCOPED_TRACE(oss.str());
+        std::string validated_path;
+        if (scenario.exp_error_.empty()) {
+            ASSERT_NO_THROW_LOG(validated_path =
+                                HooksLibrariesParser::validatePath(scenario.lib_path_, false));
+            EXPECT_EQ(validated_path, scenario.exp_path_);
+        } else {
+            ASSERT_THROW_MSG(validated_path =
+                             HooksLibrariesParser::validatePath(scenario.lib_path_, false),
+                             BadValue, scenario.exp_error_);
+        }
+    }
+}
 
 } // Anonymous namespace

src/lib/util/filesystem.cc

@@ -14,10 +14,12 @@
 #include <cstdlib>
 #include <fstream>
 #include <string>
+#include <filesystem>
 
 #include <dirent.h>
 #include <fcntl.h>
 
+using namespace isc;
 using namespace isc::util::str;
 using namespace std;
 
@@ -215,6 +217,35 @@ string TemporaryDirectory::dirName() {
     return dir_name_;
 }
 
+std::string
+FileManager::validatePath(const std::string supported_path_str, const std::string input_path_str,
+                          bool enforce_path /* = false */) {
+    std::filesystem::path supported_path(supported_path_str);
+    auto input_path = std::filesystem::path(input_path_str);
+    if (!input_path.has_filename()) {
+        isc_throw(BadValue, "path: '" << input_path.string() << "' has no filename");
+    }
+
+    auto filename = input_path.filename();
+    if (input_path.has_parent_path()) {
+        if (!enforce_path) {
+            // Security set to lax, let it fly.
+            return (input_path_str);
+        }
+
+        // We only allow absolute path equal to default. Catch an invalid path.
+        input_path.remove_filename();
+        if (input_path != (supported_path / "")) {
+            isc_throw(BadValue, "invalid path specified: '"
+                      << input_path.string() << "', supported path is '"
+                      << supported_path.string() << "'");
+        }
+    }
+
+    auto valid_path = supported_path / filename;
+    return (valid_path.string());
+}
+
 }  // namespace file
 }  // namespace util
 }  // namespace isc

src/lib/util/filesystem.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2021-2024 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2021-2025 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -16,35 +16,35 @@ namespace file {
 
 /// @brief Get the content of a regular file.
 ///
-/// \param file_name The file name.
+/// @param file_name The file name.
 ///
-/// \return The content of the file.
+/// @return The content of the file.
-/// \throw BadValue when the file can't be opened or is not a regular one.
+/// @throw BadValue when the file can't be opened or is not a regular one.
 std::string
 getContent(const std::string& file_name);
 
 /// @brief Check if there is a file or directory at the given path.
 ///
-/// \param path The path being checked.
+/// @param path The path being checked.
 ///
-/// \return True if the path points to a file or a directory, false otherwise.
+/// @return True if the path points to a file or a directory, false otherwise.
 bool
 exists(const std::string& path);
 
 /// @brief Check if there is a directory at the given path.
 ///
-/// \param path The path being checked.
+/// @param path The path being checked.
 ///
-/// \return True if the path points to a directory, false otherwise including
+/// @return True if the path points to a directory, false otherwise including
 /// if the pointed location does not exist.
 bool
 isDir(const std::string& path);
 
 /// @brief Check if there is a file at the given path.
 ///
-/// \param path The path being checked.
+/// @param path The path being checked.
 ///
-/// \return True if the path points to a file, false otherwise including
+/// @return True if the path points to a file, false otherwise including
 /// if the pointed location does not exist.
 bool
 isFile(const std::string& path);
@@ -80,35 +80,35 @@ struct Path {
     ///
     /// Counterpart for std::filesystem::path::string.
     ///
-    /// \return stored filename.
+    /// @return stored filename.
     std::string str() const;
 
     /// @brief Get the parent path.
     ///
     /// Counterpart for std::filesystem::path::parent_path.
     ///
-    /// \return parent path of current path.
+    /// @return parent path of current path.
     std::string parentPath() const;
 
     /// @brief Get the base name of the file without the extension.
     ///
     /// Counterpart for std::filesystem::path::stem.
     ///
-    /// \return the base name of current path without the extension.
+    /// @return the base name of current path without the extension.
     std::string stem() const;
 
     /// @brief Get the extension of the file.
     ///
     /// Counterpart for std::filesystem::path::extension.
     ///
-    /// \return extension of current path.
+    /// @return extension of current path.
     std::string extension() const;
 
     /// @brief Get the name of the file, extension included.
     ///
     /// Counterpart for std::filesystem::path::filename.
     ///
-    /// \return name + extension of current path.
+    /// @return name + extension of current path.
     std::string filename() const;
 
     /// @brief Identifies the extension in {replacement}, trims it, and
@@ -119,9 +119,9 @@ struct Path {
     /// The change is done in the members and {this} is returned to allow call
     /// chaining.
     ///
-    /// \param replacement The extension to replace with.
+    /// @param replacement The extension to replace with.
     ///
-    /// \return The current instance after the replacement was done.
+    /// @return The current instance after the replacement was done.
     Path& replaceExtension(std::string const& replacement = std::string());
 
     /// @brief Trims {replacement} and replaces this instance's parent path with
@@ -130,9 +130,9 @@ struct Path {
     /// The change is done in the members and {this} is returned to allow call
     /// chaining.
     ///
-    /// \param replacement The parent path to replace with.
+    /// @param replacement The parent path to replace with.
     ///
-    /// \return The current instance after the replacement was done.
+    /// @return The current instance after the replacement was done.
     Path& replaceParentPath(std::string const& replacement = std::string());
 
 private:
@@ -154,6 +154,31 @@ private:
     std::string dir_name_;
 };
 
+/// @brief Class that provides basic file related tasks.
+class FileManager {
+public:
+    /// @brief Validates a file path against a supported path.
+    ///
+    /// If the input path specifies a parent path and file name, the parent path
+    /// is validated against the supported path. If they match, the function returns
+    /// the validated path. If the input path contains only a file name the function
+    /// returns valid path using the supported path and the input path name.
+    ///
+    /// @param supported_path_str absolute path specifying the  supported path
+    /// of the file against which the input path is validated.
+    /// @param input_path_str file path to validate.
+    /// @param enforce_path enables validation against the supported path.  If false
+    /// verifies only that the path contains a file name.
+    ///
+    /// @return validated path as a string (supported path + input file name)
+    ///
+    /// @throw BadValue if the input path does not include a file name or if the
+    /// it the parent path does not path the supported path.
+    static std::string validatePath(const std::string supported_path_str,
+                                    const std::string input_path_str,
+                                    bool enforce_path = true);
+};
+
 }  // namespace file
 }  // namespace util
 }  // namespace isc

src/lib/util/tests/filesystem_unittests.cc

@@ -11,6 +11,7 @@
 #include <util/filesystem.h>
 
 #include <fstream>
+#include <list>
 #include <string>
 
 #include <gtest/gtest.h>
@@ -136,4 +137,120 @@ TEST(PathTest, replaceParentPath) {
     EXPECT_EQ("/just/some/dir/a.b", fname.str());
 }
 
+
+TEST(FileManager, validatePathEnforcePath) {
+    std::string def_path(TEST_DATA_BUILDDIR);
+    struct Scenario {
+        int line_;
+        std::string lib_path_;
+        std::string exp_path_;
+        std::string exp_error_;
+    };
+
+    std::list<Scenario> scenarios = {
+    {
+        // Invalid parent path.
+        __LINE__,
+        "/var/lib/bs/mylib.a",
+        "",
+        string("invalid path specified: '/var/lib/bs/', supported path is '" + def_path + "'")
+    },
+    {
+        // No file name.
+        __LINE__,
+        def_path + "/",
+        "",
+        string ("path: '" + def_path + "/' has no filename")
+    },
+    {
+        // File name only is valid.
+        __LINE__,
+        "mylib.a",
+        def_path + "/mylib.a",
+        ""
+    },
+    {
+        // Valid full path.
+        __LINE__,
+        def_path + "/mylib.a",
+        def_path + "/mylib.a",
+        ""
+    }
+    };
+
+    for (auto scenario : scenarios) {
+        std::ostringstream oss;
+        oss << " Scenario at line: " << scenario.line_;
+        SCOPED_TRACE(oss.str());
+        std::string validated_path;
+        if (scenario.exp_error_.empty()) {
+            ASSERT_NO_THROW_LOG(validated_path =
+                                FileManager::validatePath(def_path, scenario.lib_path_));
+            EXPECT_EQ(validated_path, scenario.exp_path_);
+        } else {
+            ASSERT_THROW_MSG(validated_path =
+                             FileManager::validatePath(def_path, scenario.lib_path_),
+                             BadValue, scenario.exp_error_);
+        }
+    }
+}
+
+TEST(FileManager, validatePathEnforcePathFalse) {
+    std::string def_path(TEST_DATA_BUILDDIR);
+    struct Scenario {
+        int line_;
+        std::string lib_path_;
+        std::string exp_path_;
+        std::string exp_error_;
+    };
+
+    std::list<Scenario> scenarios = {
+    {
+        // Invalid parent path but shouldn't care.
+        __LINE__,
+        "/var/lib/bs/mylib.a",
+        "/var/lib/bs/mylib.a",
+        ""
+    },
+    {
+        // No file name.
+        __LINE__,
+        def_path + "/",
+        "",
+        string ("path: '" + def_path + "/' has no filename")
+    },
+    {
+        // File name only is valid.
+        __LINE__,
+        "mylib.a",
+        def_path + "/mylib.a",
+        ""
+    },
+    {
+        // Valid full path.
+        __LINE__,
+        def_path + "/mylib.a",
+        def_path + "/mylib.a",
+        ""
+    }
+    };
+
+    for (auto scenario : scenarios) {
+        std::ostringstream oss;
+        oss << " Scenario at line: " << scenario.line_;
+        SCOPED_TRACE(oss.str());
+        std::string validated_path;
+        if (scenario.exp_error_.empty()) {
+            ASSERT_NO_THROW_LOG(validated_path =
+                                FileManager::validatePath(def_path, scenario.lib_path_, false));
+            EXPECT_EQ(validated_path, scenario.exp_path_);
+        } else {
+            ASSERT_THROW_MSG(validated_path =
+                             FileManager::validatePath(def_path, scenario.lib_path_, false),
+                             BadValue, scenario.exp_error_);
+        }
+    }
+}
+
+
 }  // namespace