CVE-2024-27318 (Python, published 2024-02-23T18:15Z)
Description: value: Versions of the package onnx before and including 1.15.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. The vulnerability occurs as a bypass for the patch added for CVE-2022-25882.

Severity: nan (CVSS v3 base score: )
Weakness: NVD-CWE-noinfo \u2014 Insufficient Information
There is insufficient information about the issue to classify it; details are unkown or unspecified.
Repository: https://github.com/onnx/onnx
Files changed:
onnx/checker.cc
onnx/checker.h
onnx/common/path.h
onnx/cpp2py_export.cc
onnx/external_data_helper.py
onnx/model_container.py
onnx/test/test_external_data.py
Commit message: Fix path sanitization bypass leading to arbitrary read (#5917)

Signed-off-by: liqunfu <liqun.fu@microsoft.com>
Signed-off-by: liqun Fu <liqun.fu@microsoft.com>
Co-authored-by: Justin Chu <justinchuby@users.noreply.github.com>

Vulnerable code:
#include "onnx/common/path.h"
#ifdef _WIN32
        auto file_path = std::filesystem::path(utf8str_to_wstring(entry.value()));
        if (file_path.is_absolute()) {
          fail_check(
              "Location of external TensorProto ( tensor name: ",
              tensor.name(),
              ") should be a relative path, but it is an absolute path: ",
              entry.value());
        }
        auto relative_path = file_path.lexically_normal().make_preferred().wstring();
        // Check that normalized relative path contains ".." on Windows.
        if (relative_path.find(L"..", 0) != std::string::npos) {
          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::wstring data_path = path_join(utf8str_to_wstring(ctx.get_model_dir()), relative_path);
        struct _stat64 buff;
        if (data_path.empty() || (data_path[0] != '#' && _wstat64(data_path.c_str(), &buff) != 0)) {
          fail_check(
              "Data of TensorProto ( tensor name: ",
              tensor.name(),
              ") should be stored in ",
              entry.value(),
              ", but it doesn't exist or is not accessible.");
        }
#else // POSIX
        if (entry.value().empty()) {
          fail_check("Location of external TensorProto ( tensor name: ", tensor.name(), ") should not be empty.");
        } else if (entry.value()[0] == '/') {
          fail_check(
              "Location of external TensorProto ( tensor name: ",
              tensor.name(),
              ") should be a relative path, but it is an absolute path: ",
              entry.value());
        }
        std::string relative_path = clean_relative_path(entry.value());
        // Check that normalized relative path contains ".." on POSIX
        if (relative_path.find("..", 0) != std::string::npos) {
          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);
        // use stat64 to check whether the file exists
#if defined(__APPLE__) || defined(__wasm__) || !defined(__GLIBC__)
        struct stat buffer; // APPLE, wasm and non-glic stdlibs do not have stat64
        if (data_path.empty() || (data_path[0] != '#' && stat((data_path).c_str(), &buffer) != 0)) {
#else
        struct stat64 buffer; // All POSIX under glibc except APPLE and wasm have stat64
        if (data_path.empty() || (data_path[0] != '#' && stat64((data_path).c_str(), &buffer) != 0)) {
#endif
          fail_check(
              "Data of TensorProto ( tensor name: ",
              tensor.name(),
              ") should be stored in ",
              data_path,
              ", but it doesn't exist or is not accessible.");
        }
        // Do not allow symlinks or directories.
        if (data_path.empty() || (data_path[0] != '#' && !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

  int size_required = MultiByteToWideChar(CP_UTF8, 0, utf8str.c_str(), (int)utf8str.size(), NULL, 0);
  MultiByteToWideChar(CP_UTF8, 0, utf8str.c_str(), (int)utf8str.size(), &ws_str[0], size_required);
    file_location = _sanitize_path(info.location)
    external_data_file_path = os.path.join(base_dir, file_location)

def _sanitize_path(path: str) -> str:
    """Remove path components which would allow traversing up a directory tree from a base path.

    Note: This method is currently very basic and should be expanded.
    """
    return path.lstrip("/.")


            file_location = ext_data._sanitize_path(info.location)
            external_data_file_path = os.path.join(base_dir, file_location)

Fixed code:
        resolve_external_data_location(ctx.get_model_dir(), entry.value(), tensor.name());
std::string resolve_external_data_location(
    const std::string& base_dir,
    const std::string& location,
    const std::string& tensor_name) {
#ifdef _WIN32
  auto file_path = std::filesystem::path(utf8str_to_wstring(location));
  if (file_path.is_absolute()) {
    fail_check(
        "Location of external TensorProto ( tensor name: ",
        tensor_name,
        ") should be a relative path, but it is an absolute path: ",
        location);
  }
  auto relative_path = file_path.lexically_normal().make_preferred().wstring();
  // Check that normalized relative path contains ".." on Windows.
  if (relative_path.find(L"..", 0) != std::string::npos) {
    fail_check(
        "Data of TensorProto ( tensor name: ",
        tensor_name,
        ") should be file inside the ",
        base_dir,
        ", but the '",
        location,
        "' points outside the directory");
  }
  std::wstring data_path = path_join(utf8str_to_wstring(base_dir), relative_path);
  struct _stat64 buff;
  if (data_path.empty() || (data_path[0] != '#' && _wstat64(data_path.c_str(), &buff) != 0)) {
    fail_check(
        "Data of TensorProto ( tensor name: ",
        tensor_name,
        ") should be stored in ",
        location,
        ", but it doesn't exist or is not accessible.");
  }
  return wstring_to_utf8str(data_path);
#else // POSIX
  if (location.empty()) {
    fail_check("Location of external TensorProto ( tensor name: ", tensor_name, ") should not be empty.");
  } else if (location[0] == '/') {
    fail_check(
        "Location of external TensorProto ( tensor name: ",
        tensor_name,
        ") should be a relative path, but it is an absolute path: ",
        location);
  }
  std::string relative_path = clean_relative_path(location);
  // Check that normalized relative path contains ".." on POSIX
  if (relative_path.find("..", 0) != std::string::npos) {
    fail_check(
        "Data of TensorProto ( tensor name: ",
        tensor_name,
        ") should be file inside the ",
        base_dir,
        ", but the '",
        location,
        "' points outside the directory");
  }
  std::string data_path = path_join(base_dir, relative_path);
  // use stat64 to check whether the file exists
#if defined(__APPLE__) || defined(__wasm__) || !defined(__GLIBC__)
  struct stat buffer; // APPLE, wasm and non-glic stdlibs do not have stat64
  if (data_path.empty() || (data_path[0] != '#' && stat((data_path).c_str(), &buffer) != 0)) {
#else
  struct stat64 buffer; // All POSIX under glibc except APPLE and wasm have stat64
  if (data_path.empty() || (data_path[0] != '#' && stat64((data_path).c_str(), &buffer) != 0)) {
#endif
    fail_check(
        "Data of TensorProto ( tensor name: ",
        tensor_name,
        ") should be stored in ",
        data_path,
        ", but it doesn't exist or is not accessible.");
  }
  // Do not allow symlinks or directories.
  if (data_path.empty() || (data_path[0] != '#' && !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.");
  }
  return data_path;
#endif
}

std::string resolve_external_data_location(
    const std::string& base_dir,
    const std::string& location,
    const std::string& tensor_name);
  int size_required = MultiByteToWideChar(CP_UTF8, 0, utf8str.c_str(), static_cast<int>(utf8str.size()), NULL, 0);
  MultiByteToWideChar(CP_UTF8, 0, utf8str.c_str(), static_cast<int>(utf8str.size()), &ws_str[0], size_required);
inline std::string wstring_to_utf8str(const std::wstring& ws_str) {
  if (ws_str.size() > INT_MAX) {
    fail_check("wstring_to_utf8str: string is too long for converting to UTF-8.");
  }
  int size_required =
      WideCharToMultiByte(CP_UTF8, 0, ws_str.c_str(), static_cast<int>(ws_str.size()), NULL, 0, NULL, NULL);
  std::string utf8str(size_required, 0);
  WideCharToMultiByte(
      CP_UTF8, 0, ws_str.c_str(), static_cast<int>(ws_str.size()), &utf8str[0], size_required, NULL, NULL);
  return utf8str;
}
  checker.def("_resolve_external_data_location", &checker::resolve_external_data_location);

import onnx.onnx_cpp2py_export.checker as c_checker
    external_data_file_path = c_checker._resolve_external_data_location(  # type: ignore[attr-defined]
        base_dir, info.location, tensor.name
    )
import onnx.onnx_cpp2py_export.checker as c_checker
            external_data_file_path = c_checker._resolve_external_data_location(  # type: ignore[attr-defined]
                base_dir, info.location, tensor.name
            )
import itertools
    @parameterized.parameterized.expand(itertools.product((True, False), (True, False)))
    def test_save_external_invalid_single_file_data_and_check(
        self, use_absolute_path: bool, use_model_path: bool
    ) -> None:
        model = onnx.load_model(self.model_filename, self.serialization_format)

        model_dir = os.path.join(self.temp_dir, "save_copy")
        os.mkdir(model_dir)

        traversal_external_data_dir = os.path.join(
            self.temp_dir, "invlid_external_data"
        )
        os.mkdir(traversal_external_data_dir)

        if use_absolute_path:
            traversal_external_data_location = os.path.join(
                traversal_external_data_dir, "tensors.bin"
            )
        else:
            traversal_external_data_location = "../invlid_external_data/tensors.bin"

        external_data_dir = os.path.join(self.temp_dir, "external_data")
        os.mkdir(external_data_dir)
        new_model_filepath = os.path.join(model_dir, "model.onnx")

        def convert_model_to_external_data_no_check(model: ModelProto, location: str):
            for tensor in model.graph.initializer:
                if tensor.HasField("raw_data"):
                    set_external_data(tensor, location)

        convert_model_to_external_data_no_check(
            model,
            location=traversal_external_data_location,
        )

        onnx.save_model(model, new_model_filepath, self.serialization_format)
        if use_model_path:
            with self.assertRaises(onnx.checker.ValidationError):
                _ = onnx.load_model(new_model_filepath, self.serialization_format)
        else:
            onnx_model = onnx.load_model(
                new_model_filepath, self.serialization_format, load_external_data=False
            )
            with self.assertRaises(onnx.checker.ValidationError):
                load_external_data_for_model(onnx_model, external_data_dir)