diff --git a/src/common/util/include/openvino/util/file_util.hpp b/src/common/util/include/openvino/util/file_util.hpp index 506197cd98649a..cbfc50125bcc27 100644 --- a/src/common/util/include/openvino/util/file_util.hpp +++ b/src/common/util/include/openvino/util/file_util.hpp @@ -110,6 +110,12 @@ std::string path_to_string(const Path& path) { /// \return A sanitized path std::string sanitize_path(const std::string& path); +/// \brief Check the safety of a file path by expecting no parent directory references ("..") and no symbolic links +/// references. +/// \param path A path to file +/// \return A validated path to file +const std::filesystem::path check_path_safety(const std::filesystem::path& path); + /// \brief Returns the name with extension for a given path /// \param path The path to the output file std::string get_file_name(const std::string& path); diff --git a/src/common/util/src/file_util.cpp b/src/common/util/src/file_util.cpp index 89569a8bb95f2e..9857d90fa06498 100644 --- a/src/common/util/src/file_util.cpp +++ b/src/common/util/src/file_util.cpp @@ -12,6 +12,7 @@ #include #include +#include "openvino/core/except.hpp" #include "openvino/util/common_util.hpp" #ifdef _WIN32 @@ -256,6 +257,19 @@ std::string ov::util::sanitize_path(const std::string& path) { return (start == std::string::npos) ? "" : sanitized_path.substr(start); } +const std::filesystem::path ov::util::check_path_safety(const std::filesystem::path& path) { + const bool contains_dotdot = std::any_of(path.begin(), path.end(), [](const auto& part) { + return part == ".."; + }); + + OPENVINO_ASSERT(contains_dotdot == false, + "Invalid file path: path contains parent directory reference '..': ", + path); + + OPENVINO_ASSERT(std::filesystem::is_symlink(path) == false, "Path must not refer to a symbolic link: ", path); + return path; +} + void ov::util::convert_path_win_style(std::string& path) { std::replace(path.begin(), path.end(), '/', '\\'); } diff --git a/src/core/include/openvino/pass/serialize.hpp b/src/core/include/openvino/pass/serialize.hpp index d0bb853352a25c..a7812251d6d376 100644 --- a/src/core/include/openvino/pass/serialize.hpp +++ b/src/core/include/openvino/pass/serialize.hpp @@ -19,6 +19,8 @@ namespace pass { * @brief Serialize transformation converts ov::Model into IR files * @attention * - dynamic shapes are not supported + * - any parent directory traversal (e.g. ".." in path) will be removed from output file paths + * - symbolic links are not allowed as output file paths * \ingroup ov_pass_cpp_api */ class OPENVINO_API Serialize : public ov::pass::ModelPass { diff --git a/src/core/src/pass/serialize.cpp b/src/core/src/pass/serialize.cpp index 00a13f7905f952..c7a5f32f013d40 100644 --- a/src/core/src/pass/serialize.cpp +++ b/src/core/src/pass/serialize.cpp @@ -1226,19 +1226,19 @@ void ngfunction_2_ir(pugi::xml_node& netXml, const std::filesystem::path valid_xml_path(const std::filesystem::path& path) { OPENVINO_ASSERT(path.extension() == ".xml", - "Path for xml file doesn't contains file name with 'xml' extension: \"", - path, - "\""); - return path; + "Path for xml file doesn't contains file name with 'xml' extension: ", + path); + + return ov::util::check_path_safety(std::filesystem::absolute(path)); } std::filesystem::path provide_bin_path(const std::filesystem::path& xml_path, const std::filesystem::path& bin_path) { if (bin_path.empty()) { auto path = xml_path; path.replace_extension(".bin"); - return path; + return ov::util::check_path_safety(std::filesystem::absolute(path)); } else { - return bin_path; + return ov::util::check_path_safety(std::filesystem::absolute(bin_path)); } } @@ -1297,11 +1297,11 @@ bool pass::Serialize::run_on_model(const std::shared_ptr& model) { ov::util::create_directory_recursive(m_xmlPath); std::ofstream bin_file(m_binPath, std::ios::binary); - OPENVINO_ASSERT(bin_file, "Can't open bin file: \"", m_binPath, "\""); + OPENVINO_ASSERT(bin_file, "Can't open bin file: ", m_binPath); // create xml file std::ofstream xml_file(m_xmlPath); - OPENVINO_ASSERT(xml_file, "Can't open xml file: \"", m_xmlPath, "\""); + OPENVINO_ASSERT(xml_file, "Can't open xml file: ", m_xmlPath); try { serializeFunc(xml_file, bin_file, model, m_version); diff --git a/src/core/tests/file_util.cpp b/src/core/tests/file_util.cpp index c2da68bf12b034..b5ea2555feda5e 100644 --- a/src/core/tests/file_util.cpp +++ b/src/core/tests/file_util.cpp @@ -111,6 +111,10 @@ TEST(file_util, sanitize_path) { string path = "C:\\workspace\\tensor.data"; EXPECT_STREQ("workspace\\tensor.data", ov::util::sanitize_path(path).c_str()); } + { + string path = "a/b/../tensor.data"; + EXPECT_STREQ("a/b/../tensor.data", ov::util::sanitize_path(path).c_str()); + } } using namespace testing; diff --git a/src/core/tests/pass/serialization/serialize.cpp b/src/core/tests/pass/serialization/serialize.cpp index c941bc7d81aee2..bb6cc438d280d8 100644 --- a/src/core/tests/pass/serialization/serialize.cpp +++ b/src/core/tests/pass/serialization/serialize.cpp @@ -861,3 +861,23 @@ TEST_F(UndefinedTypeDynamicTypeSerializationTests, compare_dynamic_type_undefine ASSERT_TRUE(files_equal(m_dynamic_type_out_xml_path, m_undefined_type_out_xml_path)) << "Serialized XML files are different: dynamic type vs undefined type"; } + +TEST(SerializationTest, PathSecurityValidationSymbolicLink) { + auto m = std::make_shared(ov::ResultVector{}, ov::ParameterVector{}, "empty_model"); + const auto symlink = std::filesystem::path("test_symlink"); + std::filesystem::create_symlink(std::filesystem::current_path(), symlink); + const auto out_xml_path = std::filesystem::path(symlink); + const auto out_bin_path = std::filesystem::path("../a/../test.bin"); + EXPECT_THROW(ov::pass::Serialize(out_xml_path, out_bin_path).run_on_model(m), ::ov::AssertFailure); + std::filesystem::remove(symlink); +} + +TEST(SerializationTest, PathSecurityValidationSymbolicLink2) { + auto m = std::make_shared(ov::ResultVector{}, ov::ParameterVector{}, "empty_model"); + const auto symlink = std::filesystem::path("test_symlink"); + std::filesystem::create_symlink(std::filesystem::current_path(), symlink); + const auto out_xml_path = std::filesystem::path("test.xml"); + const auto out_bin_path = std::filesystem::path(symlink); + EXPECT_THROW(ov::pass::Serialize(out_xml_path, out_bin_path).run_on_model(m), ::ov::AssertFailure); + std::filesystem::remove(symlink); +}