From: Thomas Markwalder Date: Wed, 23 Apr 2025 18:27:22 +0000 (-0400) Subject: [#3830] Hook libraries must load from default hook dir X-Git-Tag: Kea-2.7.9~78 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=4afdeb77199c649d0a2be8cb81dd06e81ff00d26;p=thirdparty%2Fkea.git [#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 --- diff --git a/src/lib/hooks/hooks_parser.cc b/src/lib/hooks/hooks_parser.cc index 62c47b13e8..cf3ee0defc 100644 --- a/src/lib/hooks/hooks_parser.cc +++ b/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 #include #include +#include #include + #include 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(); - - // 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; - } + libname = validatePath((entry_item.second)->stringValue()); // 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)); +} + } } diff --git a/src/lib/hooks/hooks_parser.h b/src/lib/hooks/hooks_parser.h index 8815d8497d..21a99afb0f 100644 --- a/src/lib/hooks/hooks_parser.h +++ b/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 diff --git a/src/lib/hooks/tests/hooks_manager_unittest.cc b/src/lib/hooks/tests/hooks_manager_unittest.cc index a36d42ce0d..90e0137dbe 100644 --- a/src/lib/hooks/tests/hooks_manager_unittest.cc +++ b/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 #include +#include #include +#include #include #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 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 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 diff --git a/src/lib/util/filesystem.cc b/src/lib/util/filesystem.cc index ea2138c86d..cb9aff8eb8 100644 --- a/src/lib/util/filesystem.cc +++ b/src/lib/util/filesystem.cc @@ -14,10 +14,12 @@ #include #include #include +#include #include #include +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 diff --git a/src/lib/util/filesystem.h b/src/lib/util/filesystem.h index 2f8a681917..d726c3178c 100644 --- a/src/lib/util/filesystem.h +++ b/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. -/// \throw BadValue when the file can't be opened or is not a regular one. +/// @return The content of the file. +/// @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 diff --git a/src/lib/util/tests/filesystem_unittests.cc b/src/lib/util/tests/filesystem_unittests.cc index 9a5020c6d7..fe35b2ac6e 100644 --- a/src/lib/util/tests/filesystem_unittests.cc +++ b/src/lib/util/tests/filesystem_unittests.cc @@ -11,6 +11,7 @@ #include #include +#include #include #include @@ -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 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 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