Model package: security, correctness, and session creation simplification#28602
Merged
Conversation
- Path traversal: validate path segments and confinement using lexical normalization (allows symlinks, blocks '..' traversal) - UAF fix: ModelPackageComponentContext owns copies of all state instead of holding a raw pointer to ModelPackageOptions - Provider options: pass effective (merged) session options to RebuildProviderListForSession so variant provider options reach EP creation - String lifetime: use context-owned caches for C string arrays - Variant order: preserve JSON key order during parsing - Device validation: reject malformed device fields, fix constraint matching logic - CPU fallback: try remaining EPs when primary variant does not match Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove stored OrtSessionOptions from ModelPackageOptions and ModelPackageComponentContext. Session creation now starts from a clean OrtSessionOptions populated only with variant metadata (session options and provider options). - Simplify GetVariantSelectionEpInfo to use the first EP in the list directly instead of skipping CPU. Remove synthetic CPU hack. - Unify RebuildProviderListForSession to use captured ep_devices from the selected VariantSelectionEpInfo, working for both explicit-factory and policy paths. - Remove HasOptions()/SessionOptions() from component context. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When session options have no explicit providers and no policy, default ep_infos to CPUExecutionProvider so variant selection works for the empty-session-options path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jambayk
added a commit
that referenced
this pull request
May 20, 2026
…tion (#28602) ## Summary This PR addresses security vulnerabilities, correctness bugs, and simplifies the session creation flow for the model package feature. ### Security & correctness fixes - **Path traversal validation**: Added `ValidatePathSegment` and `ValidatePathConfinement` to prevent directory traversal attacks via malicious package descriptors. Uses `lexically_normal()` (not `weakly_canonical()`) to support symlinked packages. - **Use-after-free fix**: `ModelPackageComponentContext` now owns all state (EP infos, provider list, devices) copied from `ModelPackageOptions`, eliminating lifetime coupling. - **Provider options propagation**: Variant provider options are now properly merged into session options and reach EP creation. - **String lifetime**: Fixed dangling pointer risk in C-string array construction. - **Variant ordering**: Variants are now evaluated in descriptor order for deterministic selection. - **Device validation**: Added bounds checking for device index in EP info resolution. - **CPU fallback**: Added fallback loop in variant selector to try CPU variant when the requested EP has no match. ### Session creation simplification - **Removed stored `OrtSessionOptions`** from both `ModelPackageOptions` and `ModelPackageComponentContext`. The session options passed to create package options are now only used to extract EP intent (factories, devices, policy) for variant selection. - **Clean-slate session creation**: `CreateSession` default path starts from a fresh `OrtSessionOptions` populated only with the selected variant's session options and provider options. - **Simplified `GetVariantSelectionEpInfo`**: Uses the first EP in the provider list directly instead of skipping CPU. Empty provider list defaults to implicit CPU. - **Unified `RebuildProviderListForSession`**: Both explicit-factory and policy paths now use `CreateIExecutionProviderFactoryForEpDevices` with captured `ep_devices`, eliminating the two-path rebuild logic. - **Removed `HasOptions()`/`SessionOptions()`** from the component context API. ### Files changed - `model_package_descriptor_parser.cc`: Path validation functions - `model_package_context.h/.cc`: Owned state, simplified rebuild - `model_package_options.h/.cc`: Removed stored session options, simplified EP resolution - `model_package_api.cc`: Clean-slate session creation - `model_package_variant_selector.cc`: CPU fallback loop - `utils.cc`: Simplified EP info extraction ### Testing - Verified with CPU-only and CUDA model packages (single and multi-variant) - E2E notebook tests pass for both ORT and GenAI integration --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR addresses security vulnerabilities, correctness bugs, and simplifies the session creation flow for the model package feature.
Security & correctness fixes
ValidatePathSegmentandValidatePathConfinementto prevent directory traversal attacks via malicious package descriptors. Useslexically_normal()(notweakly_canonical()) to support symlinked packages.ModelPackageComponentContextnow owns all state (EP infos, provider list, devices) copied fromModelPackageOptions, eliminating lifetime coupling.Session creation simplification
OrtSessionOptionsfrom bothModelPackageOptionsandModelPackageComponentContext. The session options passed to create package options are now only used to extract EP intent (factories, devices, policy) for variant selection.CreateSessiondefault path starts from a freshOrtSessionOptionspopulated only with the selected variant's session options and provider options.GetVariantSelectionEpInfo: Uses the first EP in the provider list directly instead of skipping CPU. Empty provider list defaults to implicit CPU.RebuildProviderListForSession: Both explicit-factory and policy paths now useCreateIExecutionProviderFactoryForEpDeviceswith capturedep_devices, eliminating the two-path rebuild logic.HasOptions()/SessionOptions()from the component context API.Files changed
model_package_descriptor_parser.cc: Path validation functionsmodel_package_context.h/.cc: Owned state, simplified rebuildmodel_package_options.h/.cc: Removed stored session options, simplified EP resolutionmodel_package_api.cc: Clean-slate session creationmodel_package_variant_selector.cc: CPU fallback looputils.cc: Simplified EP info extractionTesting