Skip to content

Add extra handling for invalid ORT format model. - #29048

Closed
skottmckay wants to merge 1 commit into
mainfrom
skottmckay/MSRC119613
Closed

Add extra handling for invalid ORT format model.#29048
skottmckay wants to merge 1 commit into
mainfrom
skottmckay/MSRC119613

Conversation

@skottmckay

Copy link
Copy Markdown
Contributor

Description

Improve handling for invalid ORT format model.

Motivation and Context

MSRC 119613

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 ORT-format model loading against semantically corrupt FlatBuffers content by converting previously-crashing null dereferences/assertions into clean Status failures with actionable error messages. It also adds targeted regression tests that construct minimal ORT-format FlatBuffers by hand to validate the new rejection paths.

Changes:

  • Add validation in Graph::LoadFromOrtFormat to reject NodeEdge entries that reference an unpopulated node slot.
  • Add validation in Node::LoadEdgesFromOrtFormat to reject EdgeEnd references to missing nodes.
  • Add validation for graph inputs/outputs referencing unknown NodeArg names, and add regression tests for all three cases.

Reviewed changes

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

File Description
onnxruntime/test/ir/graph_test.cc Adds regression tests that build minimal ORT-format FlatBuffers to ensure invalid graphs fail with clear errors instead of crashing.
onnxruntime/core/graph/graph.cc Adds defensive null checks and clearer error returns for corrupt ORT-format edges and graph IO NodeArg references.

@skottmckay
skottmckay enabled auto-merge (squash) June 15, 2026 04:23
@skottmckay

Copy link
Copy Markdown
Contributor Author

This overlaps with #28186. @tianleiwu is there anything from this PR worth keeping? Maybe the tests?

@skottmckay skottmckay closed this Jun 15, 2026
auto-merge was automatically disabled June 15, 2026 07:27

Pull request was closed

tianleiwu pushed a commit to tianleiwu/onnxruntime that referenced this pull request Jun 15, 2026
Add two regression tests for invalid ORT format models, covering the gaps
not already exercised by this PR's existing tests:

- RejectsEdgeEndReferencingNullNodeSlot: inner EdgeEnd referencing a
  within-range but unpopulated node slot
- RejectsGraphInputWithUnknownNodeArg: graph input referencing an
  undeclared NodeArg name

These consolidate the unique test coverage from microsoft#29048 (skottmckay) so that
PR can be closed in favor of this one.
@tianleiwu

Copy link
Copy Markdown
Contributor

@skottmckay Thanks! I've consolidated the unique test coverage from this PR into #28186 (commit 23f9aa3).

Comparing your three tests against the existing tests in 28186:

  • RejectsNodeEdgeForUnpopulatedNodeSlot → already covered by RejectsDanglingNodeEdge
  • RejectsEdgeEndForUnpopulatedNodeSlot → only the out-of-range path was covered; I added RejectsEdgeEndReferencingNullNodeSlot for the within-range null-slot case
  • RejectsGraphIoWithUnknownNodeArgName → was not covered; added as RejectsGraphInputWithUnknownNodeArg

The graph.cc hardening overlaps with what 28186 already does, so I kept 28186's implementation and just folded in the missing test coverage. This PR can be closed in favor of #28186.

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.

3 participants