Protect reading external weights from up-dir (#3098)

* introduced path sanitizing

* added tests, folders structure clean-up

* fixed test

* fixed up-dir path handling

* improved sanitizing for windows

* fix Windows test

* move sanitize impl

inference-engine/tests/functional/inference_engine/onnx_reader/models/ひらがな日本語.prototxt

@@ -23,7 +23,7 @@ graph {
     name: "A"
     external_data {
         key: "location",
-        value: "../data/tensor.data"
+        value: "data/tensor.data"
     }
     data_location: 1
   }

inference-engine/tests/functional/inference_engine/onnx_reader/onnx_reader_external_data.cpp

@@ -78,7 +78,7 @@ TEST(ONNX_Reader_Tests, ImportModelWithExternalDataFromWstringNamedFile) {
     std::string win_dir_path = ONNX_TEST_MODELS;
     std::replace(win_dir_path.begin(), win_dir_path.end(), '/', '\\');
     const std::wstring unicode_win_dir_path = FileUtils::multiByteCharToWString(win_dir_path.c_str());
-    const std::wstring path = unicode_win_dir_path + L"АБВГДЕЁЖЗИЙ\\ひらがな日本語.prototxt";
+    const std::wstring path = unicode_win_dir_path + L"ひらがな日本語.prototxt";
 
     auto cnnNetwork = ie.ReadNetwork(path, L"");
     auto function = cnnNetwork.getFunction();

ngraph/core/include/ngraph/file_util.hpp

@@ -77,5 +77,10 @@ namespace ngraph
         /// \param str A null-terminated string
         /// \return A wide-char string
         NGRAPH_API std::wstring multi_byte_char_to_wstring(const char* str);
+
+        /// \brief Remove path components which would allow traversing up a directory tree.
+        /// \param path A path to file
+        /// \return A sanitiazed path
+        NGRAPH_API std::string sanitize_path(const std::string& path);
     }
 }

ngraph/core/src/file_util.cpp

@@ -255,6 +255,15 @@ void file_util::iterate_files(const string& path,
     }
 }
 
+std::string file_util::sanitize_path(const std::string& path)
+{
+    const auto colon_pos = path.find(":");
+    const auto sanitized_path = path.substr(colon_pos == std::string::npos ? 0 : colon_pos + 1);
+    const std::string to_erase = "/.\\";
+    const auto start = sanitized_path.find_first_not_of(to_erase);
+    return (start == std::string::npos) ? "" : sanitized_path.substr(start);
+}
+
 NGRAPH_API void file_util::convert_path_win_style(std::string& path)
 {
     std::replace(path.begin(), path.end(), '/', '\\');
@@ -291,5 +300,4 @@ std::wstring file_util::multi_byte_char_to_wstring(const char* str)
     return result;
 #endif
 }
-
 #endif // ENABLE_UNICODE_PATH_SUPPORT

ngraph/frontend/onnx_import/src/core/transform.cpp

@@ -92,8 +92,10 @@ void ngraph::onnx_import::transform::update_external_data_paths(
         {
             const auto external_data_relative_path =
                 initializer_tensor.external_data(location_key_value_index).value();
+            const auto santized_external_data_relative_path =
+                file_util::sanitize_path(external_data_relative_path);
             auto external_data_full_path =
-                file_util::path_join(model_dir_path, external_data_relative_path);
+                file_util::path_join(model_dir_path, santized_external_data_relative_path);
 
 #if defined(ENABLE_UNICODE_PATH_SUPPORT) && defined(_WIN32)
             file_util::convert_path_win_style(external_data_full_path);

ngraph/test/file_util.cpp

@@ -89,3 +89,31 @@ TEST(file_util, path_join)
         EXPECT_STREQ("/test1/test2", file_util::path_join(s1, s2).c_str());
     }
 }
+
+TEST(file_util, santize_path)
+{
+    {
+        string path = "../../tensor.data";
+        EXPECT_STREQ("tensor.data", file_util::sanitize_path(path).c_str());
+    }
+    {
+        string path = "/../tensor.data";
+        EXPECT_STREQ("tensor.data", file_util::sanitize_path(path).c_str());
+    }
+    {
+        string path = "..";
+        EXPECT_STREQ("", file_util::sanitize_path(path).c_str());
+    }
+    {
+        string path = "workspace/data/tensor.data";
+        EXPECT_STREQ("workspace/data/tensor.data", file_util::sanitize_path(path).c_str());
+    }
+    {
+        string path = "..\\..\\tensor.data";
+        EXPECT_STREQ("tensor.data", file_util::sanitize_path(path).c_str());
+    }
+    {
+        string path = "C:\\workspace\\tensor.data";
+        EXPECT_STREQ("workspace\\tensor.data", file_util::sanitize_path(path).c_str());
+    }
+}

ngraph/test/models/onnx/external_data/external_data.prototxt

@@ -41,7 +41,7 @@ graph {
     name: "A"
     external_data {
         key: "location",
-        value: "../../files/onnx/external_data/tensor.data"
+        value: "tensors_data/tensor.data"
     }
     data_location: 1
   }

ngraph/test/models/onnx/external_data/external_data_different_paths.prototxt

@@ -15,7 +15,7 @@ graph {
     name: "data_a"
     external_data {
         key: "location",
-        value: "../../files/onnx/external_data/a/tensor_a.data"
+        value: "tensors_data/a/tensor_a.data"
     }
     data_location: 1
   }
@@ -25,7 +25,7 @@ graph {
     name: "data_b"
     external_data {
         key: "location",
-        value: "../../files/onnx/external_data/b/tensor_b.data"
+        value: "tensors_data/b/tensor_b.data"
     }
     data_location: 1
   }

ngraph/test/models/onnx/external_data/external_data_optional_fields.prototxt

@@ -41,7 +41,7 @@ graph {
     name: "A"
     external_data {
         key: "location",
-        value: "../../files/onnx/external_data/tensor_optional_fields.data"
+        value: "tensors_data/tensor_optional_fields.data"
     }
     external_data {
         key: "offset",

ngraph/test/models/onnx/external_data/external_data_sanitize_test.prototxt

@@ -0,0 +1,99 @@
+ir_version: 3
+producer_name: "nGraph ONNX Importer"
+graph {
+  node {
+    output: "B"
+    op_type: "Constant"
+    attribute {
+      name: "value"
+      t {
+        dims: 2
+        dims: 2
+        data_type: 1
+        float_data: 1
+        float_data: 2
+        float_data: 3
+        float_data: 4
+        name: "const_tensor"
+      }
+      type: TENSOR
+    }
+  }
+  node {
+    input: "A"
+    input: "B"
+    output: "X"
+    name: "add_node1"
+    op_type: "Add"
+  }
+  node {
+    input: "X"
+    input: "C"
+    output: "Y"
+    name: "add_node2"
+    op_type: "Add"
+  }
+  name: "test_graph"
+  initializer {
+    dims: 2
+    dims: 2
+    data_type: 1
+    name: "A"
+    external_data {
+        key: "location",
+        value: "./tensors_data/tensor.data"
+    }
+    data_location: 1
+  }
+  input {
+    name: "A"
+    type {
+      tensor_type {
+        elem_type: 1
+        shape {
+          dim {
+            dim_value: 2
+          }
+          dim {
+            dim_value: 2
+          }
+        }
+      }
+    }
+  }
+  input {
+    name: "C"
+    type {
+      tensor_type {
+        elem_type: 1
+        shape {
+          dim {
+            dim_value: 2
+          }
+          dim {
+            dim_value: 2
+          }
+        }
+      }
+    }
+  }
+  output {
+    name: "Y"
+    type {
+      tensor_type {
+        elem_type: 1
+        shape {
+          dim {
+            dim_value: 2
+          }
+          dim {
+            dim_value: 2
+          }
+        }
+      }
+    }
+  }
+}
+opset_import {
+  version: 4
+}

ngraph/test/models/onnx/external_data/external_data_two_tensors_data_in_the_same_file.prototxt

@@ -15,7 +15,7 @@ graph {
     name: "data_a"
     external_data {
         key: "location",
-        value: "../../files/onnx/external_data/multiple_tensors.data"
+        value: "tensors_data/multiple_tensors.data"
     }
     external_data {
         key: "offset",
@@ -33,7 +33,7 @@ graph {
     name: "data_b"
     external_data {
         key: "location",
-        value: "../../files/onnx/external_data/multiple_tensors.data"
+        value: "tensors_data/multiple_tensors.data"
     }
     external_data {
         key: "offset",

ngraph/test/models/onnx/external_data/inner_scope/external_data_file_in_up_dir.prototxt

@@ -0,0 +1,82 @@
+ir_version: 3
+producer_name: "nGraph ONNX Importer"
+graph {
+  node {
+    input: "A"
+    input: "B"
+    output: "Y"
+    name: "add"
+    op_type: "Add"
+  }
+  name: "test_graph"
+  initializer {
+    dims: 2
+    dims: 2
+    data_type: 1
+    name: "A"
+    external_data {
+        key: "location",
+        value: "../tensors_data/tensor.data"
+    }
+    external_data {
+        key: "offset",
+        value: "4096"
+    }
+    external_data {
+        key: "length",
+        value: "16"
+    }
+    data_location: 1
+  }
+  input {
+    name: "A"
+    type {
+      tensor_type {
+        elem_type: 1
+        shape {
+          dim {
+            dim_value: 2
+          }
+          dim {
+            dim_value: 2
+          }
+        }
+      }
+    }
+  }
+  input {
+    name: "B"
+    type {
+      tensor_type {
+        elem_type: 1
+        shape {
+          dim {
+            dim_value: 2
+          }
+          dim {
+            dim_value: 2
+          }
+        }
+      }
+    }
+  }
+  output {
+    name: "Y"
+    type {
+      tensor_type {
+        elem_type: 1
+        shape {
+          dim {
+            dim_value: 2
+          }
+          dim {
+            dim_value: 2
+          }
+        }
+      }
+    }
+  }
+}
+opset_import {
+  version: 4
+}

ngraph/test/onnx/onnx_import_external_data.in.cpp

@@ -36,7 +36,7 @@ using TestEngine = test::ENGINE_CLASS_NAME(${BACKEND_NAME});
 NGRAPH_TEST(${BACKEND_NAME}, onnx_external_data)
 {
     const auto function = onnx_import::import_onnx_model(
-        file_util::path_join(SERIALIZED_ZOO, "onnx/external_data.prototxt"));
+        file_util::path_join(SERIALIZED_ZOO, "onnx/external_data/external_data.prototxt"));
 
     auto test_case = test::TestCase<TestEngine>(function);
     test_case.add_input<float>({1.f, 2.f, 3.f, 4.f});
@@ -47,7 +47,8 @@ NGRAPH_TEST(${BACKEND_NAME}, onnx_external_data)
 
 NGRAPH_TEST(${BACKEND_NAME}, onnx_external_data_from_stream)
 {
-    std::string path = file_util::path_join(SERIALIZED_ZOO, "onnx/external_data.prototxt");
+    std::string path =
+        file_util::path_join(SERIALIZED_ZOO, "onnx/external_data/external_data.prototxt");
     std::ifstream stream{path, std::ios::in | std::ios::binary};
     ASSERT_TRUE(stream.is_open());
     const auto function = onnx_import::import_onnx_model(stream, path);
@@ -63,8 +64,8 @@ NGRAPH_TEST(${BACKEND_NAME}, onnx_external_data_from_stream)
 
 NGRAPH_TEST(${BACKEND_NAME}, onnx_external_data_optinal_fields)
 {
-    const auto function = onnx_import::import_onnx_model(
+    const auto function = onnx_import::import_onnx_model(file_util::path_join(
-        file_util::path_join(SERIALIZED_ZOO, "onnx/external_data_optional_fields.prototxt"));
+        SERIALIZED_ZOO, "onnx/external_data/external_data_optional_fields.prototxt"));
 
     auto test_case = test::TestCase<TestEngine>(function);
     test_case.add_input<float>({1.f, 2.f, 3.f, 4.f});
@@ -75,8 +76,8 @@ NGRAPH_TEST(${BACKEND_NAME}, onnx_external_data_optinal_fields)
 
 NGRAPH_TEST(${BACKEND_NAME}, onnx_external_data_in_different_paths)
 {
-    auto function = onnx_import::import_onnx_model(
+    auto function = onnx_import::import_onnx_model(file_util::path_join(
-        file_util::path_join(SERIALIZED_ZOO, "onnx/external_data_different_paths.prototxt"));
+        SERIALIZED_ZOO, "onnx/external_data/external_data_different_paths.prototxt"));
 
     auto test_case = test::TestCase<TestEngine>(function);
     // first input: {3.f}, second: {1.f, 2.f, 5.f} read from external files
@@ -89,7 +90,8 @@ NGRAPH_TEST(${BACKEND_NAME}, onnx_external_data_in_different_paths)
 NGRAPH_TEST(${BACKEND_NAME}, onnx_external_two_tensors_data_in_the_same_file)
 {
     auto function = onnx_import::import_onnx_model(file_util::path_join(
-        SERIALIZED_ZOO, "onnx/external_data_two_tensors_data_in_the_same_file.prototxt"));
+        SERIALIZED_ZOO,
+        "onnx/external_data/external_data_two_tensors_data_in_the_same_file.prototxt"));
 
     auto test_case = test::TestCase<TestEngine>(function);
     // first input: {3, 2, 1}, second: {1, 2, 3} read from external file
@@ -103,8 +105,8 @@ NGRAPH_TEST(${BACKEND_NAME}, onnx_external_invalid_external_data_exception)
 {
     try
     {
-        auto function = onnx_import::import_onnx_model(
+        auto function = onnx_import::import_onnx_model(file_util::path_join(
-            file_util::path_join(SERIALIZED_ZOO, "onnx/external_data_file_not_found.prototxt"));
+            SERIALIZED_ZOO, "onnx/external_data/external_data_file_not_found.prototxt"));
         FAIL() << "Incorrect path to external data not detected";
     }
     catch (const ngraph_error& error)
@@ -119,3 +121,37 @@ NGRAPH_TEST(${BACKEND_NAME}, onnx_external_invalid_external_data_exception)
         FAIL() << "Importing onnx model failed for unexpected reason";
     }
 }
+
+NGRAPH_TEST(${BACKEND_NAME}, onnx_external_invalid_up_dir_path)
+{
+    try
+    {
+        auto function = onnx_import::import_onnx_model(file_util::path_join(
+            SERIALIZED_ZOO,
+            "onnx/external_data/inner_scope/external_data_file_in_up_dir.prototxt"));
+        FAIL() << "Incorrect path to external data not detected";
+    }
+    catch (const ngraph_error& error)
+    {
+        EXPECT_PRED_FORMAT2(testing::IsSubstring,
+                            std::string("tensor.data, offset: 4096, "
+                                        "data_lenght: 16, sha1_digest: 0)"),
+                            error.what());
+    }
+    catch (...)
+    {
+        FAIL() << "Importing onnx model failed for unexpected reason";
+    }
+}
+
+NGRAPH_TEST(${BACKEND_NAME}, onnx_external_data_sanitize_path)
+{
+    const auto function = onnx_import::import_onnx_model(file_util::path_join(
+        SERIALIZED_ZOO, "onnx/external_data/external_data_sanitize_test.prototxt"));
+
+    auto test_case = test::TestCase<TestEngine>(function);
+    test_case.add_input<float>({1.f, 2.f, 3.f, 4.f});
+    test_case.add_expected_output<float>(Shape{2, 2}, {3.f, 6.f, 9.f, 12.f});
+
+    test_case.run();
+}