[rpms/onnx] test-1.21.0: Backport of fixes for CVE-2024-27318 and CVE-2024-27319

[rpms/onnx] test-1.21.0: Backport of fixes for CVE-2024-27318 and CVE-2024-27319

[Alejandro Alvarez Ayllon]

A new commit has been pushed.

Repo   : rpms/onnx
Branch : test-1.21.0
Commit : 0bc03015f99f80a102f41c3720e2e65295c91683
Author : Alejandro Alvarez Ayllon <a.alvarezayllon@gmail.com>
Date   : 2024-02-24T16:18:04+01:00
Stats  : +493/-1 in 4 file(s)
URL    : https://src.fedoraproject.org/rpms/onnx/c/0bc03015f99f80a102f41c3720e2e65295c91683?branch=test-1.21.0

Log:
Backport of fixes for CVE-2024-27318 and CVE-2024-27319

---
diff --git a/0005-Fix-path-sanitization-bypass-leading-to-arbitrary-re.patch b/0005-Fix-path-sanitization-bypass-leading-to-arbitrary-re.patch
new file mode 100644
index 0000000..67d71bb
--- /dev/null
+++ b/0005-Fix-path-sanitization-bypass-leading-to-arbitrary-re.patch
@@ -0,0 +1,380 @@
+From da2c24b4a9ae802efe8768962308c784cda871e2 Mon Sep 17 00:00:00 2001
+From: liqun Fu <liqfu@microsoft.com>
+Date: Mon, 19 Feb 2024 11:12:40 -0800
+Subject: [PATCH 5/6] 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>
+(cherry picked from commit 66b7fb630903fdcf3e83b6b6d56d82e904264a20)
+---
+ onnx/checker.cc                 | 168 +++++++++++++++++---------------
+ onnx/checker.h                  |   5 +
+ onnx/common/path.h              |  15 ++-
+ onnx/cpp2py_export.cc           |   2 +
+ onnx/external_data_helper.py    |  15 +--
+ onnx/test/test_external_data.py |  49 ++++++++++
+ 6 files changed, 161 insertions(+), 93 deletions(-)
+
+diff --git a/onnx/checker.cc b/onnx/checker.cc
+index 37d6abd3..38e544be 100644
+--- a/onnx/checker.cc
++++ b/onnx/checker.cc
+@@ -4,7 +4,6 @@
+ 
+ #include "onnx/checker.h"
+ #include "onnx/common/file_utils.h"
+-#include "onnx/common/path.h"
+ #include "onnx/defs/schema.h"
+ #include "onnx/defs/tensor_proto_util.h"
+ #include "onnx/proto_utils.h"
+@@ -129,85 +128,7 @@ void check_tensor(const TensorProto& tensor, const CheckerContext& ctx) {
+     for (const StringStringEntryProto& entry : tensor.external_data()) {
+       if (entry.has_key() && entry.has_value() && entry.key() == "location") {
+         has_location = true;
+-#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 (_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__)
+-        struct stat buffer; // APPLE does not have stat64
+-        if (stat((data_path).c_str(), &buffer) != 0) {
+-#else
+-        struct stat64 buffer; // All POSIX except APPLE have stat64
+-        if (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 (!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
++        resolve_external_data_location(ctx.get_model_dir(), entry.value(), tensor.name());
+       }
+     }
+     if (!has_location) {
+@@ -1045,6 +966,93 @@ void check_model(const ModelProto& model, bool full_check) {
+   }
+ }
+ 
++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::set<std::string> experimental_ops = {
+     "ATen",
+     "Affine",
+diff --git a/onnx/checker.h b/onnx/checker.h
+index 54fe19aa..6f1d47b2 100644
+--- a/onnx/checker.h
++++ b/onnx/checker.h
+@@ -149,6 +149,11 @@ void check_model_local_functions(
+ void check_model(const ModelProto& model, bool full_check = false);
+ void check_model(const std::string& model_path, bool full_check = false);
+ 
++std::string resolve_external_data_location(
++    const std::string& base_dir,
++    const std::string& location,
++    const std::string& tensor_name);
++
+ bool check_is_experimental_op(const NodeProto& node);
+ 
+ } // namespace checker
+diff --git a/onnx/common/path.h b/onnx/common/path.h
+index d06eef90..fc692152 100644
+--- a/onnx/common/path.h
++++ b/onnx/common/path.h
+@@ -29,11 +29,22 @@ inline std::wstring utf8str_to_wstring(const std::string& utf8str) {
+   if (utf8str.size() > INT_MAX) {
+     fail_check("utf8str_to_wstring: string is too long for converting to wstring.");
+   }
+-  int size_required = MultiByteToWideChar(CP_UTF8, 0, utf8str.c_str(), (int)utf8str.size(), NULL, 0);
++  int size_required = MultiByteToWideChar(CP_UTF8, 0, utf8str.c_str(), static_cast<int>(utf8str.size()), NULL, 0);
+   std::wstring ws_str(size_required, 0);
+-  MultiByteToWideChar(CP_UTF8, 0, utf8str.c_str(), (int)utf8str.size(), &ws_str[0], size_required);
++  MultiByteToWideChar(CP_UTF8, 0, utf8str.c_str(), static_cast<int>(utf8str.size()), &ws_str[0], size_required);
+   return ws_str;
+ }
++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;
++}
+ 
+ #else
+ std::string path_join(const std::string& origin, const std::string& append);
+diff --git a/onnx/cpp2py_export.cc b/onnx/cpp2py_export.cc
+index 656f8fa7..dd53b197 100644
+--- a/onnx/cpp2py_export.cc
++++ b/onnx/cpp2py_export.cc
+@@ -538,6 +538,8 @@ PYBIND11_MODULE(onnx_cpp2py_export, onnx_cpp2py_export) {
+       "path"_a,
+       "full_check"_a = false);
+ 
++  checker.def("_resolve_external_data_location", &checker::resolve_external_data_location);
++
+   // Submodule `version_converter`
+   auto version_converter = onnx_cpp2py_export.def_submodule("version_converter");
+   version_converter.doc() = "VersionConverter submodule";
+diff --git a/onnx/external_data_helper.py b/onnx/external_data_helper.py
+index cfb97d06..992c324b 100644
+--- a/onnx/external_data_helper.py
++++ b/onnx/external_data_helper.py
+@@ -8,6 +8,7 @@ import uuid
+ from itertools import chain
+ from typing import Callable, Iterable, Optional
+ 
++import onnx.onnx_cpp2py_export.checker as c_checker
+ from onnx.onnx_pb import AttributeProto, GraphProto, ModelProto, TensorProto
+ 
+ 
+@@ -39,9 +40,9 @@ def load_external_data_for_tensor(tensor: TensorProto, base_dir: str) -> None:
+         base_dir: directory that contains the external data.
+     """
+     info = ExternalDataInfo(tensor)
+-    file_location = _sanitize_path(info.location)
+-    external_data_file_path = os.path.join(base_dir, file_location)
+-
++    external_data_file_path = c_checker._resolve_external_data_location(  # type: ignore[attr-defined]
++        base_dir, info.location, tensor.name
++    )
+     with open(external_data_file_path, "rb") as data_file:
+         if info.offset:
+             data_file.seek(info.offset)
+@@ -254,14 +255,6 @@ def _get_attribute_tensors(onnx_model_proto: ModelProto) -> Iterable[TensorProto
+     yield from _get_attribute_tensors_from_graph(onnx_model_proto.graph)
+ 
+ 
+-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("/.")
+-
+-
+ def _is_valid_filename(filename: str) -> bool:
+     """Utility to check whether the provided filename is valid."""
+     exp = re.compile('^[^<>:;,?"*|/]+$')
+diff --git a/onnx/test/test_external_data.py b/onnx/test/test_external_data.py
+index b4303b4a..c7d03d78 100644
+--- a/onnx/test/test_external_data.py
++++ b/onnx/test/test_external_data.py
+@@ -1,6 +1,9 @@
+ # Copyright (c) ONNX Project Contributors
+ 
+ # SPDX-License-Identifier: Apache-2.0
++from __future__ import annotations
++
++import itertools
+ import os
+ import shutil
+ import tempfile
+@@ -186,6 +189,52 @@ class TestLoadExternalDataSingleFile(TestLoadExternalDataBase):
+         attribute_tensor = new_model.graph.node[0].attribute[0].t
+         self.assertTrue(np.allclose(to_array(attribute_tensor), self.attribute_value))
+ 
++    @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)
++
+ 
+ class TestSaveAllTensorsAsExternalData(TestLoadExternalDataBase):
+     def setUp(self) -> None:
+-- 
+2.43.0
+

diff --git a/0006-Fix-Out-of-bounds-read-due-to-lack-of-string-termina.patch b/0006-Fix-Out-of-bounds-read-due-to-lack-of-string-termina.patch
new file mode 100644
index 0000000..a7821e7
--- /dev/null
+++ b/0006-Fix-Out-of-bounds-read-due-to-lack-of-string-termina.patch
@@ -0,0 +1,54 @@
+From 69502bca61dafaa40b4315ea1cb03db96ae55304 Mon Sep 17 00:00:00 2001
+From: liqun Fu <liqfu@microsoft.com>
+Date: Fri, 9 Feb 2024 14:45:49 -0800
+Subject: [PATCH 6/6] Fix Out of bounds read due to lack of string termination
+ in assert (#5918)
+
+Signed-off-by: liqunfu <liqun.fu@microsoft.com>
+Co-authored-by: G. Ramalingam <grama@microsoft.com>
+(cherry picked from commit 08a399ba75a805b7813ab8936b91d0e274b08287)
+---
+ onnx/common/assertions.cc | 15 ++++++++++-----
+ 1 file changed, 10 insertions(+), 5 deletions(-)
+
+diff --git a/onnx/common/assertions.cc b/onnx/common/assertions.cc
+index a21e55da..2099003d 100644
+--- a/onnx/common/assertions.cc
++++ b/onnx/common/assertions.cc
+@@ -8,6 +8,7 @@
+ // Adventurous users should note that the APIs will probably change.
+ 
+ #include "onnx/common/assertions.h"
++#include <array>
+ #include <cstdarg>
+ #include <cstdio>
+ #include "onnx/common/common.h"
+@@ -15,16 +16,20 @@
+ namespace ONNX_NAMESPACE {
+ 
+ std::string barf(const char* fmt, ...) {
+-  char msg[2048];
++  constexpr size_t buffer_size = 2048;
++  std::array<char, buffer_size> msg{};
+   va_list args;
+ 
+   va_start(args, fmt);
+-  // Although vsnprintf might have vulnerability issue while using format string with overflowed length,
+-  // it should be safe here to use fixed length for buffer "msg". No further checking is needed.
+-  vsnprintf(msg, 2048, fmt, args);
++
++  // use fixed length for buffer "msg" to avoid buffer overflow
++  vsnprintf(static_cast<char*>(msg.data()), msg.size() - 1, fmt, args);
++
++  // ensure null-terminated string to avoid out of bounds read
++  msg.back() = '\0';
+   va_end(args);
+ 
+-  return std::string(msg);
++  return std::string(msg.data());
+ }
+ 
+ void throw_assert_error(std::string& msg) {
+-- 
+2.43.0
+

diff --git a/0007-Fix-test_external_data.patch b/0007-Fix-test_external_data.patch
new file mode 100644
index 0000000..8e11216
--- /dev/null
+++ b/0007-Fix-test_external_data.patch
@@ -0,0 +1,50 @@
+From d2dae07c2bae6714d2496f9eb125314bd20eacda Mon Sep 17 00:00:00 2001
+From: Alejandro Alvarez Ayllon <a.alvarezayllon@gmail.com>
+Date: Sat, 24 Feb 2024 15:15:58 +0100
+Subject: [PATCH 7/7] Fix test_external_data
+
+---
+ onnx/test/test_external_data.py | 9 +++++----
+ 1 file changed, 5 insertions(+), 4 deletions(-)
+
+diff --git a/onnx/test/test_external_data.py b/onnx/test/test_external_data.py
+index c7d03d78..c4376159 100644
+--- a/onnx/test/test_external_data.py
++++ b/onnx/test/test_external_data.py
+@@ -5,6 +5,7 @@ from __future__ import annotations
+ 
+ import itertools
+ import os
++import parameterized
+ import shutil
+ import tempfile
+ import unittest
+@@ -193,7 +194,7 @@ class TestLoadExternalDataSingleFile(TestLoadExternalDataBase):
+     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 = onnx.load_model(self.model_filename)
+ 
+         model_dir = os.path.join(self.temp_dir, "save_copy")
+         os.mkdir(model_dir)
+@@ -224,13 +225,13 @@ class TestLoadExternalDataSingleFile(TestLoadExternalDataBase):
+             location=traversal_external_data_location,
+         )
+ 
+-        onnx.save_model(model, new_model_filepath, self.serialization_format)
++        onnx.save_model(model, new_model_filepath)
+         if use_model_path:
+             with self.assertRaises(onnx.checker.ValidationError):
+-                _ = onnx.load_model(new_model_filepath, self.serialization_format)
++                _ = onnx.load_model(new_model_filepath)
+         else:
+             onnx_model = onnx.load_model(
+-                new_model_filepath, self.serialization_format, load_external_data=False
++                new_model_filepath, load_external_data=False
+             )
+             with self.assertRaises(onnx.checker.ValidationError):
+                 load_external_data_for_model(onnx_model, external_data_dir)
+-- 
+2.43.0
+

diff --git a/onnx.spec b/onnx.spec
index 7bf4cc8..61abd0e 100644
--- a/onnx.spec
+++ b/onnx.spec
@@ -1,6 +1,6 @@
 Name:       onnx
 Version:    1.14.1
-Release:    1%{?dist}
+Release:    2%{?dist}
 Summary:    Open standard for machine learning interoperability
 License:    Apache-2.0
 
@@ -16,6 +16,11 @@ Patch2:     0002-Use-system-protobuf-and-require-parameterized.patch
 Patch3:     0003-Let-pyproject_wheel-use-binaries-from-cmake_build.patch
 # Add fixes for use with onnxruntime
 Patch4:     0004-Add-fixes-for-use-with-onnxruntime.patch
+# Backport of fix for CVE-2024-27318
+Patch5:     0005-Fix-path-sanitization-bypass-leading-to-arbitrary-re.patch
+# Backport of fix for CVE-2024-27319
+Patch6:     0006-Fix-Out-of-bounds-read-due-to-lack-of-string-termina.patch
+Patch7:     0007-Fix-test_external_data.patch
 
 # https://bugzilla.redhat.com/show_bug.cgi?id=2212096
 ExcludeArch:    s390x
@@ -110,6 +115,9 @@ export LD_LIBRARY_PATH=%{buildroot}/%{_libdir}
 %{_bindir}/check-node
 
 %changelog
+* Sat Feb 24 2024 Alejandro Alvarez Ayllon <a.alvarezayllon@gmail.com> - 1.14.1-2
+- Backport of fixes for CVE-2024-27318 and CVE-2024-27319
+
 * Wed Feb 21 2024 Diego Herrera C <dherrera@redhat.com>- 1.14.1-1
 - Release 1.14.1