From 19282c4c72ff5ddd679b9074576a9db0abe51102 Mon Sep 17 00:00:00 2001 From: Xu Xing Date: Thu, 13 Aug 2026 15:34:46 +0800 Subject: [PATCH] [Core] Add nesting-depth limit to If::validate_and_infer_types (CWE-674) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### Details: The constant-condition branch of `If::validate_and_infer_types` (if.cpp:109-113) calls `validate_and_infer_type_body`, which at `multi_subgraph_base.cpp:164` invokes `body->validate_nodes_and_infer_types()` with no depth counter or recursion limit. If the body contains another constant-condition If, the same path is re-entered, producing one native C++ stack frame per nesting level. A sufficiently deep chain overflows the call stack and crashes the process (SIGSEGV / EXCEPTION_STACK_OVERFLOW). This path is distinct from the non-constant branch (:124-128): a model whose If ops all carry constant conditions never enters that branch, yet still recurses unboundedly here. The IR frontend (pugixml) imposes no XML nesting cap, making this fully exploitable via a crafted IR model. Fix: introduce a shared `ValidationDepthGuard` RAII class (`validation_depth_guard.hpp`) with a per-op-type `static thread_local` counter and `OV_VALIDATION_DEPTH_GUARD` macro. In `If::validate_and_infer_types`, a single line guards both the constant and non-constant paths against nesting deeper than `kMaxValidationDepth = 64`. ```cpp // validation_depth_guard.hpp — reusable by Loop and other subgraph ops OV_VALIDATION_DEPTH_GUARD(this, "If"); ``` Tests added: - `type_prop.if_nested_constant_condition_exceeds_max_depth_throws` (ov_core_unit_tests) — directly validates the depth guard throws `NodeValidationFailure` at depth 1024. - `IRFrontendTestsIf.nested_if_at_max_depth_loads` (ov_ir_frontend_tests) — boundary: depth-64 nested If IR loads successfully. - `IRFrontendTestsIf.nested_if_depth_limit_is_rejected` (ov_ir_frontend_tests) — end-to-end: depth-1024 nested If IR is rejected via `core.read_model()`. ### Tickets: - *CVS-192797* ### AI Assistance: - *AI assistance used: yes* - *Bug root cause was found by AI.* --- .../op/util/validation_depth_guard.hpp | 47 +++ src/core/src/op/if.cpp | 3 + src/core/tests/type_prop/if.cpp | 37 +++ src/frontends/ir/tests/if_deserialization.cpp | 274 ++++++++++++++++++ 4 files changed, 361 insertions(+) create mode 100644 src/core/include/openvino/op/util/validation_depth_guard.hpp create mode 100644 src/frontends/ir/tests/if_deserialization.cpp diff --git a/src/core/include/openvino/op/util/validation_depth_guard.hpp b/src/core/include/openvino/op/util/validation_depth_guard.hpp new file mode 100644 index 00000000000..e71bcf419af --- /dev/null +++ b/src/core/include/openvino/op/util/validation_depth_guard.hpp @@ -0,0 +1,47 @@ +// Copyright (C) 2018-2026 Intel Corporation +// SPDX-License-Identifier: Apache-2.0 +// + +#pragma once + +#include + +#include "openvino/core/node.hpp" + +namespace ov { +namespace op { +namespace util { + +constexpr size_t kMaxValidationDepth = 64; + +/// RAII depth guard for subgraph validation recursion (CWE-674). +/// Increments a per-op-type thread_local counter on construction, checks +/// the limit, and decrements on destruction. +class ValidationDepthGuard { +public: + ValidationDepthGuard(size_t& depth, const Node* node, const char* op_name) : m_depth(depth) { + ++m_depth; + NODE_VALIDATION_CHECK(node, + m_depth <= kMaxValidationDepth, + op_name, + " nesting depth exceeds the maximum allowed limit of ", + kMaxValidationDepth); + } + ~ValidationDepthGuard() { + --m_depth; + } + ValidationDepthGuard(const ValidationDepthGuard&) = delete; + ValidationDepthGuard& operator=(const ValidationDepthGuard&) = delete; + +private: + size_t& m_depth; +}; + +// Usage: OV_VALIDATION_DEPTH_GUARD(this, "If"); +#define OV_VALIDATION_DEPTH_GUARD(node, op_name) \ + static thread_local size_t ov_validation_depth_ = 0; \ + ov::op::util::ValidationDepthGuard ov_depth_guard_(ov_validation_depth_, node, op_name) + +} // namespace util +} // namespace op +} // namespace ov diff --git a/src/core/src/op/if.cpp b/src/core/src/op/if.cpp index 16458f80c6d..d0c41a0da9a 100644 --- a/src/core/src/op/if.cpp +++ b/src/core/src/op/if.cpp @@ -11,6 +11,7 @@ #include "openvino/core/graph_util.hpp" #include "openvino/core/validation_util.hpp" #include "openvino/op/util/multi_subgraph_base.hpp" +#include "openvino/op/util/validation_depth_guard.hpp" #include "openvino/reference/if.hpp" ov::op::v8::If::If() : MultiSubGraphOp(2) {} @@ -78,6 +79,8 @@ bool ov::op::v8::If::visit_attributes(AttributeVisitor& visitor) { void ov::op::v8::If::validate_and_infer_types() { OV_OP_SCOPE(v8_If_validate_and_infer_types); + OV_VALIDATION_DEPTH_GUARD(this, "If"); + NODE_VALIDATION_CHECK(this, m_bodies.size() == 2, "If contains incorrect number of bodies:", m_bodies.size()); NODE_VALIDATION_CHECK(this, diff --git a/src/core/tests/type_prop/if.cpp b/src/core/tests/type_prop/if.cpp index ad8d6e0ddc5..92b7292cfd3 100644 --- a/src/core/tests/type_prop/if.cpp +++ b/src/core/tests/type_prop/if.cpp @@ -6,6 +6,7 @@ #include +#include "common_test_utils/test_assertions.hpp" #include "common_test_utils/type_prop.hpp" #include "openvino/op/add.hpp" #include "openvino/op/constant.hpp" @@ -17,6 +18,7 @@ using namespace std; using namespace ov; +using testing::HasSubstr; TEST(type_prop, if_simple_test) { // That which we iterate over @@ -447,3 +449,38 @@ TEST(type_prop, if_invalid_false_body) { EXPECT_EQ(then_op_res->get_element_type(), ov::element::dynamic); EXPECT_EQ(else_op_res->get_element_type(), ov::element::f16); } + +static shared_ptr make_nested_if(size_t depth) { + auto cond = make_shared(element::boolean, Shape{1}, true); + auto if_op = make_shared(cond); + + // then/else bodies + auto then_p = make_shared(element::f32, PartialShape::dynamic()); + shared_ptr then_out; + if (depth > 0) { + // nest another If inside the then-body + auto inner = make_nested_if(depth - 1); + then_out = inner; + } else { + then_out = then_p; + } + auto then_res = make_shared(then_out); + auto then_body = make_shared(OutputVector{then_res}, ParameterVector{then_p}); + + auto else_p = make_shared(element::f32, PartialShape::dynamic()); + auto else_res = make_shared(else_p); + auto else_body = make_shared(OutputVector{else_res}, ParameterVector{else_p}); + + if_op->set_then_body(then_body); + if_op->set_else_body(else_body); + if_op->set_input(if_op->input_value(0), then_p, else_p); + if_op->set_output(then_res, else_res); + return if_op; +} + +TEST(type_prop, if_nested_constant_condition_exceeds_max_depth_throws) { + const size_t excessive_depth = 1024; + OV_EXPECT_THROW(std::ignore = make_nested_if(excessive_depth), + ov::NodeValidationFailure, + HasSubstr("nesting depth exceeds")); +} diff --git a/src/frontends/ir/tests/if_deserialization.cpp b/src/frontends/ir/tests/if_deserialization.cpp new file mode 100644 index 00000000000..d66cba869fd --- /dev/null +++ b/src/frontends/ir/tests/if_deserialization.cpp @@ -0,0 +1,274 @@ +// Copyright (C) 2018-2026 Intel Corporation +// SPDX-License-Identifier: Apache-2.0 +// + +#include + +#include "frontend_test.hpp" +using testing::HasSubstr; + +class IRFrontendTestsIf : public ::testing::Test, public IRFrontendTestsImpl { +protected: + void SetUp() override {} + + void TearDown() override { + RemoveTemporalFiles(); + } +}; + +// Build an IR XML string containing an If op whose then-body itself contains +// another If, nested to the requested depth. Every condition is a Const(true), +// which forces the constant-condition branch of If::validate_and_infer_types. +// +// The topology at each nesting level is: +// Const(bool) ──┐ +// Parameter ────┤ If ── Result +// │ +// then_body: contains the next-level If (or just Parameter→Result at leaf) +// else_body: Parameter → Result (pass-through) +static std::string generate_nested_if_xml(size_t depth) { + // Each nesting level needs unique layer IDs. We use a simple counter. + int next_id = 0; + auto alloc_id = [&]() { + return next_id++; + }; + + // Recursive lambda: returns the ...... + // fragment for a single If level and all its children. Also returns + // the layer-id of the top-level If (or the pass-through Parameter at + // the leaf) so the parent can wire edges. + struct Fragment { + std::string layers; + std::string edges; + int output_layer_id; + int output_port_id; + }; + + // Forward-declare for the recursive lambda. + std::function build_level; + + build_level = [&](size_t remaining_depth, int input_param_id) -> Fragment { + if (remaining_depth == 0) { + // Leaf: just return the input parameter info (the body is a + // trivial Parameter→Result; the Parameter is already created by + // the caller as part of the body). + return {"", "", input_param_id, 0}; + } + + std::ostringstream layers, edges; + + // -- Const condition (true) -- + int cond_id = alloc_id(); + layers << R"( )" + << R"( + + + + + +)"; + + // -- The If node -- + int if_id = alloc_id(); + + // then-body: Parameter(f32, dynamic) → [nested-If or pass-through] → Result + int then_param_id = alloc_id(); + int then_result_id = alloc_id(); + + // else-body: Parameter(f32, dynamic) → Result (trivial pass-through) + int else_param_id = alloc_id(); + int else_result_id = alloc_id(); + + // Build the nested content for then-body + auto inner = build_level(remaining_depth - 1, then_param_id); + + // Construct then-body layers + std::ostringstream then_layers; + then_layers << R"( )" + << R"( + + + + 1 + + + +)"; + then_layers << inner.layers; + + then_layers << R"( )" + << R"( + + + 1 + + + +)"; + + // then-body edges + std::ostringstream then_edges; + then_edges << inner.edges; + // Connect inner output to Result + int src_layer = (remaining_depth - 1 > 0) ? inner.output_layer_id : then_param_id; + int src_port = inner.output_port_id; + then_edges << R"( +)"; + + // Construct else-body layers (trivial pass-through) + std::ostringstream else_layers; + else_layers << R"( )" + << R"( + + + + 1 + + + +)"; + else_layers << R"( )" + << R"( + + + 1 + + + +)"; + + std::ostringstream else_edges; + else_edges << R"( +)"; + + // -- If layer -- + // Input ports: 0=condition, 1=data + // Output ports: 2=output + layers << R"( )" + << R"( + + + + + 1 + + + + + 1 + + + + + + + + + + + + +)" << then_layers.str() << R"( + +)" << then_edges.str() << R"( + + + +)" << else_layers.str() << R"( + +)" << else_edges.str() << R"( + + +)"; + + // Edges: Const→If:0, input_param→If:1 + edges << R"( +)"; + edges << R"( +)"; + + return {layers.str(), edges.str(), if_id, /*output port*/ 2}; + }; + + // Top-level model: Parameter → nested If chain → Result + int top_param_id = alloc_id(); + auto frag = build_level(depth, top_param_id); + int top_result_id = alloc_id(); + + std::ostringstream xml; + xml << R"( + + + + + + + 1 + + + +)" << frag.layers + << R"( + + + 1 + + + + + +)" << frag.edges + << R"( + + +)"; + + return xml.str(); +} + +// Boundary: depth=64 equals kMaxIfValidationDepth and must load successfully. +TEST_F(IRFrontendTestsIf, nested_if_at_max_depth_loads) { + std::string xmlModel = generate_nested_if_xml(64); + // Single-byte bin for the Const(true) boolean values + std::vector buffer(1, 1); + createTemporalModelFile(xmlModel, buffer); + + std::shared_ptr model; + OV_ASSERT_NO_THROW(model = core.read_model(xmlFileName, binFileName)); + ASSERT_NE(model, nullptr); +} + +// CWE-674: depth=1024 far exceeds kMaxIfValidationDepth (64); without the fix this +// would overflow the stack, with the fix it throws ov::Exception. +TEST_F(IRFrontendTestsIf, nested_if_depth_limit_is_rejected) { + std::string xmlModel = generate_nested_if_xml(1024); + std::vector buffer(1, 1); + createTemporalModelFile(xmlModel, buffer); + + OV_EXPECT_THROW(core.read_model(xmlFileName, binFileName), + ov::Exception, + HasSubstr("nesting depth exceeds the maximum")); +}