CVE-2023-2356: Relative Path Traversal (CWE-23)
value: Relative Path Traversal in GitHub repository mlflow/mlflow prior to 2.3.1.
CWE description: The product uses external input to construct a pathname that should be within a restricted directory, but it does not properly neutralize sequences such as ".." that can resolve to a location that is outside of that directory.
CVSS v3 base score: 7.5
Published: 2023-04-28T00:15Z
Repository: https://github.com/mlflow/mlflow
Language: Python
Files changed: mlflow/server/handlers.py, tests/tracking/test_rest_tracking.py
Commit date: 2023-04-20 13:51:51 +0000
Commit message: Disable ability to provide relative paths in sources (#8281)

* Disable ability to provide relative paths in sources

Signed-off-by: Ben Wilson <benjamin.wilson@databricks.com>

* no relative paths allowed

Signed-off-by: Ben Wilson <benjamin.wilson@databricks.com>

---------

Signed-off-by: Ben Wilson <benjamin.wilson@databricks.com>
Diff: diff --git a/mlflow/server/handlers.py b/mlflow/server/handlers.py
index d7423f50f..7c2f55183 100644
--- a/mlflow/server/handlers.py
+++ b/mlflow/server/handlers.py
@@ -1321,10 +1321,40 @@ def _delete_registered_model_tag():
         name=request_message.name, key=request_message.key
     )
     return _wrap_response(DeleteRegisteredModelTag.Response())
 
 
+def _validate_non_local_source_contains_relative_paths(source: str):
+    """
+    Validation check to ensure that sources that are provided that conform to the schemes:
+    http, https, or mlflow-artifacts do not contain relative path designations that are intended
+    to access local file system paths on the tracking server.
+
+    Example paths that this validation function is intended to find and raise an Exception if
+    passed:
+    "mlflow-artifacts://host:port/../../../../"
+    "http://host:port/api/2.0/mlflow-artifacts/artifacts/../../../../"
+    "https://host:port/api/2.0/mlflow-artifacts/artifacts/../../../../"
+    "/models/artifacts/../../../"
+    "s3:/my_bucket/models/path/../../other/path"
+    "file://path/to/../../../../some/where/you/should/not/be"
+    """
+    source_path = urllib.parse.urlparse(source).path
+    resolved_source = pathlib.Path(source_path).resolve().as_posix()
+    # NB: drive split is specifically for Windows since WindowsPath.resolve() will append the
+    # drive path of the pwd to a given path. We don't care about the drive here, though.
+    _, resolved_path = os.path.splitdrive(resolved_source)
+
+    if resolved_path != source_path:
+        raise MlflowException(
+            f"Invalid model version source: '{source}'. If supplying a source as an http, https, "
+            "local file path, ftp, objectstore, or mlflow-artifacts uri, an absolute path must be "
+            "provided without relative path references present. Please provide an absolute path.",
+            INVALID_PARAMETER_VALUE,
+        )
+
+
 def _validate_source(source: str, run_id: str) -> None:
     if is_local_uri(source):
         if run_id:
             store = _get_tracking_store()
             run = store.get_run(run_id)
@@ -1350,10 +1380,14 @@ def _validate_source(source: str, run_id: str) -> None:
             f"the {MLFLOW_ALLOW_FILE_URI_AS_MODEL_VERSION_SOURCE.name} environment variable to "
             "True.",
             INVALID_PARAMETER_VALUE,
         )
 
+    # Checks if relative paths are present in the source (a security threat). If any are present,
+    # raises an Exception.
+    _validate_non_local_source_contains_relative_paths(source)
+
 
 @catch_mlflow_exception
 @_disable_if_artifacts_only
 def _create_model_version():
     request_message = _get_request_message(
diff --git a/tests/tracking/test_rest_tracking.py b/tests/tracking/test_rest_tracking.py
index c8f4a2b6c..5748590d8 100644
--- a/tests/tracking/test_rest_tracking.py
+++ b/tests/tracking/test_rest_tracking.py
@@ -1043,11 +1043,11 @@ def test_get_metric_history_bulk_calls_optimized_impl_when_expected(monkeypatch,
             max_results=25000,
         )
 
 
 def test_create_model_version_with_path_source(mlflow_client):
-    name = "mode"
+    name = "model"
     mlflow_client.create_registered_model(name)
     exp_id = mlflow_client.create_experiment("test")
     run = mlflow_client.create_run(experiment_id=exp_id)
 
     response = requests.post(
@@ -1082,10 +1082,104 @@ def test_create_model_version_with_path_source(mlflow_client):
     )
     assert response.status_code == 400
     assert "To use a local path as a model version" in response.json()["message"]
 
 
+def test_create_model_version_with_non_local_source(mlflow_client, monkeypatch):
+    name = "model"
+    mlflow_client.create_registered_model(name)
+    exp_id = mlflow_client.create_experiment("test")
+    run = mlflow_client.create_run(experiment_id=exp_id)
+
+    response = requests.post(
+        f"{mlflow_client.tracking_uri}/api/2.0/mlflow/model-versions/create",
+        json={
+            "name": name,
+            "source": run.info.artifact_uri[len("file://") :],
+            "run_id": run.info.run_id,
+        },
+    )
+    assert response.status_code == 200
+
+    # Test that remote uri's supplied as a source with absolute paths work fine
+    response = requests.post(
+        f"{mlflow_client.tracking_uri}/api/2.0/mlflow/model-versions/create",
+        json={
+            "name": name,
+            "source": "mlflow-artifacts:/models",
+            "run_id": run.info.run_id,
+        },
+    )
+    assert response.status_code == 200
+
+    response = requests.post(
+        f"{mlflow_client.tracking_uri}/api/2.0/mlflow/model-versions/create",
+        json={
+            "name": name,
+            "source": "mlflow-artifacts://host:9000/models",
+            "run_id": run.info.run_id,
+        },
+    )
+    assert response.status_code == 200
+
+    # Test that invalid remote uri's cannot be created
+    response = requests.post(
+        f"{mlflow_client.tracking_uri}/api/2.0/mlflow/model-versions/create",
+        json={
+            "name": name,
+            "source": "mlflow-artifacts://host:9000/models/../../../",
+            "run_id": run.info.run_id,
+        },
+    )
+    assert response.status_code == 400
+    assert "If supplying a source as an http, https," in response.json()["message"]
+
+    response = requests.post(
+        f"{mlflow_client.tracking_uri}/api/2.0/mlflow/model-versions/create",
+        json={
+            "name": name,
+            "source": "http://host:9000/models/../../../",
+            "run_id": run.info.run_id,
+        },
+    )
+    assert response.status_code == 400
+    assert "If supplying a source as an http, https," in response.json()["message"]
+
+    response = requests.post(
+        f"{mlflow_client.tracking_uri}/api/2.0/mlflow/model-versions/create",
+        json={
+            "name": name,
+            "source": "https://host/api/2.0/mlflow-artifacts/artifacts/../../../",
+            "run_id": run.info.run_id,
+        },
+    )
+    assert response.status_code == 400
+    assert "If supplying a source as an http, https," in response.json()["message"]
+
+    response = requests.post(
+        f"{mlflow_client.tracking_uri}/api/2.0/mlflow/model-versions/create",
+        json={
+            "name": name,
+            "source": "s3a://my_bucket/api/2.0/mlflow-artifacts/artifacts/../../../",
+            "run_id": run.info.run_id,
+        },
+    )
+    assert response.status_code == 400
+    assert "If supplying a source as an http, https," in response.json()["message"]
+
+    response = requests.post(
+        f"{mlflow_client.tracking_uri}/api/2.0/mlflow/model-versions/create",
+        json={
+            "name": name,
+            "source": "ftp://host:8888/api/2.0/mlflow-artifacts/artifacts/../../../",
+            "run_id": run.info.run_id,
+        },
+    )
+    assert response.status_code == 400
+    assert "If supplying a source as an http, https," in response.json()["message"]
+
+
 def test_create_model_version_with_file_uri(mlflow_client):
     name = "test"
     mlflow_client.create_registered_model(name)
     exp_id = mlflow_client.create_experiment("test")
     run = mlflow_client.create_run(experiment_id=exp_id)

Vulnerable code:     name = "mode"
Fixed code: def _validate_non_local_source_contains_relative_paths(source: str):
    """
    Validation check to ensure that sources that are provided that conform to the schemes:
    http, https, or mlflow-artifacts do not contain relative path designations that are intended
    to access local file system paths on the tracking server.

    Example paths that this validation function is intended to find and raise an Exception if
    passed:
    "mlflow-artifacts://host:port/../../../../"
    "http://host:port/api/2.0/mlflow-artifacts/artifacts/../../../../"
    "https://host:port/api/2.0/mlflow-artifacts/artifacts/../../../../"
    "/models/artifacts/../../../"
    "s3:/my_bucket/models/path/../../other/path"
    "file://path/to/../../../../some/where/you/should/not/be"
    """
    source_path = urllib.parse.urlparse(source).path
    resolved_source = pathlib.Path(source_path).resolve().as_posix()
    # NB: drive split is specifically for Windows since WindowsPath.resolve() will append the
    # drive path of the pwd to a given path. We don't care about the drive here, though.
    _, resolved_path = os.path.splitdrive(resolved_source)

    if resolved_path != source_path:
        raise MlflowException(
            f"Invalid model version source: '{source}'. If supplying a source as an http, https, "
            "local file path, ftp, objectstore, or mlflow-artifacts uri, an absolute path must be "
            "provided without relative path references present. Please provide an absolute path.",
            INVALID_PARAMETER_VALUE,
        )


    # Checks if relative paths are present in the source (a security threat). If any are present,
    # raises an Exception.
    _validate_non_local_source_contains_relative_paths(source)

    name = "model"
def test_create_model_version_with_non_local_source(mlflow_client, monkeypatch):
    name = "model"
    mlflow_client.create_registered_model(name)
    exp_id = mlflow_client.create_experiment("test")
    run = mlflow_client.create_run(experiment_id=exp_id)

    response = requests.post(
        f"{mlflow_client.tracking_uri}/api/2.0/mlflow/model-versions/create",
        json={
            "name": name,
            "source": run.info.artifact_uri[len("file://") :],
            "run_id": run.info.run_id,
        },
    )
    assert response.status_code == 200

    # Test that remote uri's supplied as a source with absolute paths work fine
    response = requests.post(
        f"{mlflow_client.tracking_uri}/api/2.0/mlflow/model-versions/create",
        json={
            "name": name,
            "source": "mlflow-artifacts:/models",
            "run_id": run.info.run_id,
        },
    )
    assert response.status_code == 200

    response = requests.post(
        f"{mlflow_client.tracking_uri}/api/2.0/mlflow/model-versions/create",
        json={
            "name": name,
            "source": "mlflow-artifacts://host:9000/models",
            "run_id": run.info.run_id,
        },
    )
    assert response.status_code == 200

    # Test that invalid remote uri's cannot be created
    response = requests.post(
        f"{mlflow_client.tracking_uri}/api/2.0/mlflow/model-versions/create",
        json={
            "name": name,
            "source": "mlflow-artifacts://host:9000/models/../../../",
            "run_id": run.info.run_id,
        },
    )
    assert response.status_code == 400
    assert "If supplying a source as an http, https," in response.json()["message"]

    response = requests.post(
        f"{mlflow_client.tracking_uri}/api/2.0/mlflow/model-versions/create",
        json={
            "name": name,
            "source": "http://host:9000/models/../../../",
            "run_id": run.info.run_id,
        },
    )
    assert response.status_code == 400
    assert "If supplying a source as an http, https," in response.json()["message"]

    response = requests.post(
        f"{mlflow_client.tracking_uri}/api/2.0/mlflow/model-versions/create",
        json={
            "name": name,
            "source": "https://host/api/2.0/mlflow-artifacts/artifacts/../../../",
            "run_id": run.info.run_id,
        },
    )
    assert response.status_code == 400
    assert "If supplying a source as an http, https," in response.json()["message"]

    response = requests.post(
        f"{mlflow_client.tracking_uri}/api/2.0/mlflow/model-versions/create",
        json={
            "name": name,
            "source": "s3a://my_bucket/api/2.0/mlflow-artifacts/artifacts/../../../",
            "run_id": run.info.run_id,
        },
    )
    assert response.status_code == 400
    assert "If supplying a source as an http, https," in response.json()["message"]

    response = requests.post(
        f"{mlflow_client.tracking_uri}/api/2.0/mlflow/model-versions/create",
        json={
            "name": name,
            "source": "ftp://host:8888/api/2.0/mlflow-artifacts/artifacts/../../../",
            "run_id": run.info.run_id,
        },
    )
    assert response.status_code == 400
    assert "If supplying a source as an http, https," in response.json()["message"]