Skip to content

Conversation

@mlukasze
Copy link
Contributor

@mlukasze mlukasze commented Feb 3, 2026

Fix Integer Overflow Vulnerability in XML Deserialization

Summary

This PR addresses a critical security vulnerability in XML model deserialization where unsigned integer overflow in the expression offset + size could allow malicious XML files to bypass bounds checking and cause out-of-bounds memory access.

Vulnerability Details

The vulnerable code performed bounds checking using:

if (m_weights->size() < offset + size)
    OPENVINO_THROW("Incorrect weights in bin file!");

When both offset and size are large size_t values, their sum can overflow, wrapping around to a small number that incorrectly passes the bounds check.

Attack Example:

offset = SIZE_MAX - 50
size = 100
offset + size = 49 (after overflow)

If m_weights->size() = 1000:
  Check: 1000 < 49 → FALSE (check passes incorrectly)
  Actual memory access: at offset SIZE_MAX - 50 (out of bounds)

Solution

The fix replaces the overflow-prone check with safe component-wise validation:

if (offset > m_weights->size() || size > m_weights->size() - offset)
    OPENVINO_THROW("Incorrect weights in bin file!");

Mathematical Proof of Correctness

Goal: Verify that offset + size <= m_weights->size() without computing offset + size

Proof:

  1. First, check offset <= m_weights->size()

    • If this fails, the condition is violated
    • If this passes, then m_weights->size() - offset is well-defined and non-negative
  2. Next, check size <= m_weights->size() - offset

    • Rearranging: size + offset <= m_weights->size()
    • This is equivalent to our goal: offset + size <= m_weights->size()
  3. No overflow can occur because:

    • We never compute offset + size
    • The subtraction m_weights->size() - offset is safe due to the first check
    • All comparisons are performed on valid, non-overflowing values

Logical Equivalence:

(offset > m_weights->size() || size > m_weights->size() - offset)
≡ ¬(offset <= m_weights->size() && size <= m_weights->size() - offset)
≡ ¬(offset + size <= m_weights->size())    [when overflow-free]
≡ (offset + size > m_weights->size())      [the intended check]

Changes Made

Fixed Files:

  1. src/core/xml_util/src/xml_deserialize_util.cpp (lines 840, 907)

    • String tensor deserialization bounds check
    • Constant buffer deserialization bounds check
  2. src/plugins/intel_cpu/src/utils/graph_serializer/deserializer.cpp (line 322)

    • Intel CPU plugin deserializer bounds check

Test Coverage:

  • Added overflow_protection_offset_size test in src/core/tests/xml_util/custom_ir.cpp
  • Validates protection against overflow attack scenarios
  • Verifies legitimate operations continue to work correctly

Testing

The new test covers three scenarios:

  1. Overflow attack with offset = SIZE_MAX - 50, size = 100
  2. Large values near SIZE_MAX/2 for both offset and size
  3. Valid small values to ensure normal operation

All scenarios correctly throw exceptions for invalid cases and succeed for valid ones.

Impact

  • Security: Eliminates integer overflow vulnerability in XML parsing
  • Compatibility: No breaking changes for legitimate XML files
  • Performance: Negligible impact (same number of comparisons)

Ticket ID: CVS-180563

Fixed potential security vulnerability where offset + size could overflow, allowing malicious XML files to bypass bounds checking. The fix uses safe arithmetic that checks components individually to prevent overflow.

Affected locations:

- src/core/xml_util/src/xml_deserialize_util.cpp (lines 840, 907)

- src/plugins/intel_cpu/src/utils/graph_serializer/deserializer.cpp (line 322)

Added comprehensive test coverage to validate overflow protection.

Ticket ID: CVS-180563
@mlukasze mlukasze requested review from a team as code owners February 3, 2026 12:23
@github-actions github-actions bot added category: Core OpenVINO Core (aka ngraph) category: CPU OpenVINO CPU plugin labels Feb 3, 2026
EXPECT_TRUE(is_valid) << error_msg;
}

TEST_F(CustomIRTest, overflow_protection_offset_size) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test will pass without any code change. Looks like changes in xml_utils are not required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category: Core OpenVINO Core (aka ngraph) category: CPU OpenVINO CPU plugin

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants