Skip to content

fix potential out of boundary issue when initializer a SVMClassifier#27699

Merged
xadupre merged 9 commits intomainfrom
xadupre/svm109373
Mar 30, 2026
Merged

fix potential out of boundary issue when initializer a SVMClassifier#27699
xadupre merged 9 commits intomainfrom
xadupre/svm109373

Conversation

@xadupre
Copy link
Copy Markdown
Member

@xadupre xadupre commented Mar 17, 2026

Description

If the ONNX file is malformed, it could lead to an incorrect memory access. This change enforces that does not happen.

Motivation and Context

security issue

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

You can commit the suggested changes from lintrunner.

Comment thread onnxruntime/test/providers/cpu/ml/svmclassifier_test.cc Outdated
Comment thread onnxruntime/test/providers/cpu/ml/svmclassifier_test.cc Outdated
Comment thread onnxruntime/test/providers/cpu/ml/svmclassifier_test.cc Outdated
Comment thread onnxruntime/test/providers/cpu/ml/svmclassifier_test.cc Outdated
Comment thread onnxruntime/test/providers/cpu/ml/svmclassifier_test.cc
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens the CPU ML SVMClassifier implementation against malformed ONNX models that could otherwise trigger out-of-bounds memory access during scoring, and adds a regression test to validate the failure behavior.

Changes:

  • Add a runtime bounds check in SVMClassifier::ComputeImpl to prevent out-of-range access into the coefficients_ buffer.
  • Add a negative test that constructs an intentionally malformed SVMClassifier configuration and asserts the operator fails with an expected error substring.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File Description
onnxruntime/core/providers/cpu/ml/svmclassifier.cc Adds an ORT_ENFORCE check intended to prevent coefficient indexing from going out of bounds for malformed models.
onnxruntime/test/providers/cpu/ml/svmclassifier_test.cc Adds a regression test that exercises malformed attribute sizing and expects a failure.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment thread onnxruntime/core/providers/cpu/ml/svmclassifier.cc Outdated
Comment thread onnxruntime/test/providers/cpu/ml/svmclassifier_test.cc Outdated
Comment thread onnxruntime/test/providers/cpu/ml/svmclassifier_test.cc Outdated
Comment thread onnxruntime/test/providers/cpu/ml/svmclassifier_test.cc Outdated
Comment thread onnxruntime/core/providers/cpu/ml/svmclassifier.cc Outdated
xadupre and others added 7 commits March 18, 2026 17:41
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@xadupre xadupre enabled auto-merge (squash) March 20, 2026 17:00
@xadupre xadupre requested a review from yuslepukhin March 20, 2026 17:21
@yuslepukhin
Copy link
Copy Markdown
Member

yuslepukhin commented Mar 20, 2026

The vulnerability report describes three independent OOB access sites. The current fix only addresses one of them. See comments.

Design issue: validation at inference time vs. construction time
The current fix validates at inference time (inside ComputeImpl), incurring overhead on every inference call. Since rho_, coefficients_, proba_, and probb_ are model attributes (immutable after construction), all validation should happen in the constructor instead. This:

  • Catches malformed models at load time (fail-fast)
  • Eliminates per-inference overhead
  • Is the standard pattern used elsewhere in ORT

Comment thread onnxruntime/core/providers/cpu/ml/svmclassifier.cc
@yuslepukhin
Copy link
Copy Markdown
Member

yuslepukhin commented Mar 20, 2026

      float val1 = sigmoid_probability(classifier_scores[index], proba_[index], probb_[index]);

index iterates from 0 to num_classifiers - 1.
If proba_ or probb_ are non-empty but have fewer than num_classifiers entries, this is an OOB heap read.
There is no validation at all for these arrays. #Resolved


Refers to: onnxruntime/core/providers/cpu/ml/svmclassifier.cc:327 in 74f0639. [](commit_id = 74f0639, deletion_comment = True)

Comment thread onnxruntime/test/providers/cpu/ml/svmclassifier_test.cc Outdated
Copy link
Copy Markdown
Member

@yuslepukhin yuslepukhin left a comment

Choose a reason for hiding this comment

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

🕐

Copy link
Copy Markdown
Member

@yuslepukhin yuslepukhin left a comment

Choose a reason for hiding this comment

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

:shipit:

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants