fix: validate ensemble composing model names against path traversal#500
Open
LinZiyuu wants to merge 1 commit into
Open
Conversation
ValidateModelName() rejects '..' and '/' in a model name, but it is only applied to the top-level requested model in LoadUnloadModel(). The composing model names of an ensemble are taken from the model config and fed into the polling loop, where each name is joined onto the repository path (JoinPath) and passed to FileExists()/ReadTextProto(). A composing name such as "../../etc" therefore escapes the model repository root, letting a model-load request read config files outside the repository and probe arbitrary host paths via the load response. Validate every composing step model name with ValidateModelName() before it is polled, matching the existing top-level check. Legitimate composing names are ordinary model names and are unaffected. Signed-off-by: LinZiyuu <linziyu0205@163.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.
This PR validates ensemble composing model names against path traversal, extending the existing top-level model-name check to the composing models an ensemble references.
Previously,
ValidateModelName()(which rejects..and/) was applied only to the top-level requested model inLoadUnloadModel(). An ensemble's composing model names are read from the model config and fed into the polling loop, where each name is joined onto the repository path (JoinPath) and passed toFileExists()/ReadTextProto(). A composing name such as../../etctherefore escaped the model repository root: a model-load request could make the server readconfig.pbtxtfiles outside the repository and probe arbitrary host paths through the differing load response.This change validates every composing step model name with
ValidateModelName()before it is polled, matching the existing top-level check. Legitimate composing names are ordinary model names and are unaffected.Reproduced on
nvcr.io/nvidia/tritonserver:26.04-py3(CPU,--model-control-mode=explicit): loading an ensemble whose stepmodel_namewas../secretmade the server read and parse/secret/config.pbtxt(a path mounted outside the repository), and a step../../../../etcmade it enumerate/etc; the load response distinguished existing from non-existing targets. After this change the load is rejected with "model name must not contain path traversal characters".Relates to the model-name validation hardening in #472 / #481.