CVE-2022-25882 (C++, published 2023-01-26T21:15Z)
Description: value: Versions of the package onnx before 1.13.0 are vulnerable to Directory Traversal as the external_data field of the tensor proto can have a path to the file which is outside the model current directory or user-provided directory, for example "../../../etc/passwd"
Severity: nan (CVSS v3 base score: 7.5)
Weakness: CWE-22 \u2014 Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal')
The product uses external input to construct a pathname that is intended to identify a file or directory that is located underneath a restricted parent directory, but the product does not properly neutralize special elements within the pathname that can cause the pathname to resolve to a location that is outside of the restricted directory.
Repository: https://github.com/onnx/onnx
Files changed:
onnx/checker.cc
onnx/common/path.cc
onnx/common/path.h
onnx/test/cpp/common_path_test.cc
onnx/test/test_external_data.py
Commit message: Do not allow to read tensor's external_data outside the model directory (#4400)

* Not allow to read tensor external_data outside the model directory

Signed-off-by: jnovikov <johnnovikov0@gmail.com>

* Fix formatting errors

Signed-off-by: jnovikov <johnnovikov0@gmail.com>

* Disable segfaulty test

Signed-off-by: jnovikov <johnnovikov0@gmail.com>

* Fix cpp tests

Signed-off-by: jnovikov <johnnovikov0@gmail.com>

* Fix UB while removing ../

Signed-off-by: jnovikov <johnnovikov0@gmail.com>

* Fix clang-format

Signed-off-by: jnovikov <johnnovikov0@gmail.com>

* Check for symlinks only on POSIX systems

Signed-off-by: jnovikov <johnnovikov0@gmail.com>

* Add specific to Windows external_data test

Signed-off-by: jnovikov <johnnovikov0@gmail.com>

* Change specific Windows external_data test decorator tofix mypy

Signed-off-by: jnovikov <johnnovikov0@gmail.com>

* Remove unused pathlib

Signed-off-by: jnovikov <johnnovikov0@gmail.com>

Signed-off-by: jnovikov <johnnovikov0@gmail.com>

Vulnerable code:
        std::string data_path = path_join(ctx.get_model_dir(), entry.value());

        convert_model_to_external_data(self.model, size_threshold=0, all_tensors_to_one_file=True, location=external_data_file)
        convert_model_to_external_data(self.model, size_threshold=0, all_tensors_to_one_file=False, convert_attribute=False)
        convert_model_to_external_data(self.model, size_threshold=0, all_tensors_to_one_file=False, convert_attribute=True)
        convert_model_to_external_data(self.model, size_threshold=0, convert_attribute=False, all_tensors_to_one_file=False)
            dims=self.large_data.shape, vals=self.large_data.tobytes(), raw=True)
            dims=shape_data.shape, vals=shape_data.tobytes(), raw=True)
        onnx.save_model(self.model, self.model_file_path, save_as_external_data=True, all_tensors_to_one_file=False, size_threshold=0)
            model_without_external_data, strict_mode=True)

Fixed code:
        std::string relative_path = clean_relative_path(entry.value());
        // Check that normalized relative path starts with "../" or "..\" on windows.
        if (relative_path.rfind(".." + k_preferred_path_separator, 0) == 0) {
          fail_check(
              "Data of TensorProto ( tensor name: ",
              tensor.name(),
              ") should be file inside the ",
              ctx.get_model_dir(),
              ", but the '",
              entry.value(),
              "' points outside the directory");
        }

        std::string data_path = path_join(ctx.get_model_dir(), relative_path);
#ifdef _WIN32
#else // POSIX
      //  Do not allow symlinks or directories.
        if (!S_ISREG(buffer.st_mode)) {
          fail_check(
              "Data of TensorProto ( tensor name: ",
              tensor.name(),
              ") should be stored in ",
              data_path,
              ", but it is not regular file.");
        }
#endif
bool is_path_separator(char c) {
  // Windows accept / as path separator.
  if (k_preferred_path_separator == "\\") {
    return c == '\\' || c == '/';
  }

  return c == k_preferred_path_separator[0];
}

void normalize_separator(std::string& path) {
  char preferred_sep = k_preferred_path_separator[0];
  if (preferred_sep == '/') {
    // Do nothing on linux.
    return;
  }

  for (size_t i = 0; i < path.size(); i++) {
    if (is_path_separator(path[i]) && path[i] != preferred_sep) {
      path[i] = preferred_sep;
    }
  }
}

std::string clean_relative_path(const std::string& path) {
  if (path.empty()) {
    return ".";
  }

  std::string out;

  char sep = k_preferred_path_separator[0];
  size_t n = path.size();

  size_t r = 0;
  size_t dotdot = 0;

  while (r < n) {
    if (is_path_separator(path[r])) {
      r++;
      continue;
    }

    if (path[r] == '.' && (r + 1 == n || is_path_separator(path[r + 1]))) {
      r++;
      continue;
    }

    if (path[r] == '.' && path[r + 1] == '.' && (r + 2 == n || is_path_separator(path[r + 2]))) {
      r += 2;

      if (out.size() > dotdot) {
        while (out.size() > dotdot && !is_path_separator(out.back())) {
          out.pop_back();
        }
        if (!out.empty())
          out.pop_back();
      } else {
        if (!out.empty()) {
          out.push_back(sep);
        }

        out.push_back('.');
        out.push_back('.');
        dotdot = out.size();
      }

      continue;
    }

    if (!out.empty() && out.back() != sep) {
      out.push_back(sep);
    }

    for (; r < n && !is_path_separator(path[r]); r++) {
      out.push_back(path[r]);
    }
  }

  if (out.empty()) {
    out.push_back('.');
  }

  // Use 1 separator in path.
  normalize_separator(out);

  return out;
}

void normalize_separator(std::string& path);
std::string clean_relative_path(const std::string& path);
/*
 * SPDX-License-Identifier: Apache-2.0
 */

#include <list>
#include <utility>
#include "gtest/gtest.h"

#include "onnx/common/path.h"

using namespace ONNX_NAMESPACE;

namespace ONNX_NAMESPACE {
namespace Test {
namespace {
std::string fix_sep(std::string path) {
  std::string out = path;
  normalize_separator(out);
  return out;
}
} // namespace

TEST(PathTest, CleanRelativePathTest) {
  // Already normal.
  EXPECT_EQ(clean_relative_path("abc"), fix_sep("abc"));
  EXPECT_EQ(clean_relative_path("abc/def"), fix_sep("abc/def"));
  EXPECT_EQ(clean_relative_path("a/b/c"), fix_sep("a/b/c"));
  EXPECT_EQ(clean_relative_path("."), fix_sep("."));
  EXPECT_EQ(clean_relative_path(".."), fix_sep(".."));
  EXPECT_EQ(clean_relative_path("../.."), fix_sep("../.."));
  EXPECT_EQ(clean_relative_path("../../abc"), fix_sep("../../abc"));
  // Remove leading slash
  EXPECT_EQ(clean_relative_path("/abc"), fix_sep("abc"));
  EXPECT_EQ(clean_relative_path("/"), fix_sep("."));
  // Remove trailing slash
  EXPECT_EQ(clean_relative_path("abc/"), fix_sep("abc"));
  EXPECT_EQ(clean_relative_path("abc/def/"), fix_sep("abc/def"));
  EXPECT_EQ(clean_relative_path("a/b/c/"), fix_sep("a/b/c"));
  EXPECT_EQ(clean_relative_path("./"), fix_sep("."));
  EXPECT_EQ(clean_relative_path("../"), fix_sep(".."));
  EXPECT_EQ(clean_relative_path("../../"), fix_sep("../.."));
  EXPECT_EQ(clean_relative_path("/abc/"), fix_sep("abc"));
  // Remove doubled slash
  EXPECT_EQ(clean_relative_path("abc//def//ghi"), fix_sep("abc/def/ghi"));
  EXPECT_EQ(clean_relative_path("//abc"), fix_sep("abc"));
  EXPECT_EQ(clean_relative_path("///abc"), fix_sep("abc"));
  EXPECT_EQ(clean_relative_path("//abc//"), fix_sep("abc"));
  EXPECT_EQ(clean_relative_path("abc//"), fix_sep("abc"));
  // Remove . elements
  EXPECT_EQ(clean_relative_path("abc/./def"), fix_sep("abc/def"));
  EXPECT_EQ(clean_relative_path("/./abc/def"), fix_sep("abc/def"));
  EXPECT_EQ(clean_relative_path("abc/."), fix_sep("abc"));
  // Remove .. elements
  EXPECT_EQ(clean_relative_path("abc/def/ghi/../jkl"), fix_sep("abc/def/jkl"));
  EXPECT_EQ(clean_relative_path("abc/def/../ghi/../jkl"), fix_sep("abc/jkl"));
  EXPECT_EQ(clean_relative_path("abc/def/.."), fix_sep("abc"));
  EXPECT_EQ(clean_relative_path("abc/def/../.."), fix_sep("."));
  EXPECT_EQ(clean_relative_path("/abc/def/../.."), fix_sep("."));
  EXPECT_EQ(clean_relative_path("abc/def/../../.."), fix_sep(".."));
  EXPECT_EQ(clean_relative_path("/abc/def/../../.."), fix_sep(".."));
  EXPECT_EQ(clean_relative_path("abc/def/../../../ghi/jkl/../../../mno"), fix_sep("../../mno"));
  EXPECT_EQ(clean_relative_path("/../abc"), fix_sep("../abc"));
  // Combinations
  EXPECT_EQ(clean_relative_path("abc/./../def"), fix_sep("def"));
  EXPECT_EQ(clean_relative_path("abc//./../def"), fix_sep("def"));
  EXPECT_EQ(clean_relative_path("abc/../../././../def"), fix_sep("../../def"));
}

} // namespace Test
} // namespace ONNX_NAMESPACE
        convert_model_to_external_data(self.model, size_threshold=0, all_tensors_to_one_file=True,
                                       location=external_data_file)
        convert_model_to_external_data(self.model, size_threshold=0, all_tensors_to_one_file=False,
                                       convert_attribute=False)
        convert_model_to_external_data(self.model, size_threshold=0, all_tensors_to_one_file=False,
                                       convert_attribute=True)
        convert_model_to_external_data(self.model, size_threshold=0, convert_attribute=False,
                                       all_tensors_to_one_file=False)
                                        dims=self.large_data.shape, vals=self.large_data.tobytes(), raw=True)
                                        dims=shape_data.shape, vals=shape_data.tobytes(), raw=True)
        onnx.save_model(self.model, self.model_file_path, save_as_external_data=True, all_tensors_to_one_file=False,
                        size_threshold=0)
                          model_without_external_data, strict_mode=True)
class TestNotAllowToLoadExternalDataOutsideModelDirectory(TestLoadExternalDataBase):
    """Essential test to check that onnx (validate) C++ code will not allow to load external_data outside the model
    directory. """

    def create_external_data_tensor(self, value: List[Any], tensor_name: str) -> TensorProto:
        tensor = from_array(np.array(value))
        tensor.name = tensor_name

        set_external_data(tensor, location="../../file.bin")

        tensor.ClearField('raw_data')
        tensor.data_location = onnx.TensorProto.EXTERNAL
        return tensor

    def test_check_model(self) -> None:
        """We only test the model validation as onnxruntime uses this to load the model. """
        with self.assertRaises(onnx.checker.ValidationError):
            checker.check_model(self.model_filename)


@pytest.mark.skipif(os.name != 'nt', reason='Skip Windows test')
class TestNotAllowToLoadExternalDataOutsideModelDirectoryOnWindows(TestLoadExternalDataBase):
    """Essential test to check that onnx (validate) C++ code will not allow to load external_data outside the model
    directory. """

    def create_external_data_tensor(self, value: List[Any], tensor_name: str) -> TensorProto:
        tensor = from_array(np.array(value))
        tensor.name = tensor_name

        set_external_data(tensor, location="..\\..\\file.bin")

        tensor.ClearField('raw_data')
        tensor.data_location = onnx.TensorProto.EXTERNAL
        return tensor

    def test_check_model(self) -> None:
        """We only test the model validation as onnxruntime uses this to load the model. """
        with self.assertRaises(onnx.checker.ValidationError):
            checker.check_model(self.model_filename)