CVE-2024-3848: Path Traversal: '\..\filename' (CWE-29)
value: A path traversal vulnerability exists in mlflow/mlflow version 2.11.0, identified as a bypass for the previously addressed CVE-2023-6909. The vulnerability arises from the application's handling of artifact URLs, where a '#' character can be used to insert a path into the fragment, effectively skipping validation. This allows an attacker to construct a URL that, when processed, ignores the protocol scheme and uses the provided path for filesystem access. As a result, an attacker can read arbitrary files, including sensitive information such as SSH and cloud keys, by exploiting the way the application converts the URL into a filesystem path. The issue stems from insufficient validation of the fragment portion of the URL, leading to arbitrary file read through path traversal.
CWE description: The product uses external input to construct a pathname that should be within a restricted directory, but it does not properly neutralize '\..\filename' (leading backslash dot dot) sequences that can resolve to a location that is outside of that directory.
Published: 2024-05-16T09:15Z
Repository: https://github.com/mlflow/mlflow
Language: Python
Files changed: mlflow/server/handlers.py, tests/server/test_handlers.py
Commit date: 2024-03-12 11:32:00 +0000
Commit message: Fix Local File Read/Path Traversal bypass (#11376)

Signed-off-by: Weichen Xu <weichen.xu@databricks.com>
Diff: diff --git a/mlflow/server/handlers.py b/mlflow/server/handlers.py
index 6d32040023..6dd78c5edd 100644
--- a/mlflow/server/handlers.py
+++ b/mlflow/server/handlers.py
@@ -598,12 +598,16 @@ def _create_experiment():
 
     tags = [ExperimentTag(tag.key, tag.value) for tag in request_message.tags]
 
     # Validate query string in artifact location to prevent attacks
     parsed_artifact_locaion = urllib.parse.urlparse(request_message.artifact_location)
+    if parsed_artifact_locaion.fragment:
+        raise MlflowException(
+            "'artifact_location' URL can't include fragment part.",
+            error_code=INVALID_PARAMETER_VALUE,
+        )
     validate_query_string(parsed_artifact_locaion.query)
-
     experiment_id = _get_tracking_store().create_experiment(
         request_message.name, request_message.artifact_location, tags
     )
     response_message = CreateExperiment.Response()
     response_message.experiment_id = experiment_id
@@ -1724,13 +1728,16 @@ 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)
             source = pathlib.Path(local_file_uri_to_path(source)).resolve()
-            run_artifact_dir = pathlib.Path(local_file_uri_to_path(run.info.artifact_uri)).resolve()
-            if run_artifact_dir in [source, *source.parents]:
-                return
+            if is_local_uri(run.info.artifact_uri):
+                run_artifact_dir = pathlib.Path(
+                    local_file_uri_to_path(run.info.artifact_uri)
+                ).resolve()
+                if run_artifact_dir in [source, *source.parents]:
+                    return
 
         raise MlflowException(
             f"Invalid model version source: '{source}'. To use a local path as a model version "
             "source, the run_id request parameter has to be specified and the local path has to be "
             "contained within the artifact directory of the run specified by the run_id.",
diff --git a/tests/server/test_handlers.py b/tests/server/test_handlers.py
index cbc760f474..f4d6d76b17 100644
--- a/tests/server/test_handlers.py
+++ b/tests/server/test_handlers.py
@@ -64,10 +64,11 @@ from mlflow.server.handlers import (
     _set_registered_model_alias,
     _set_registered_model_tag,
     _transition_stage,
     _update_model_version,
     _update_registered_model,
+    _validate_source,
     catch_mlflow_exception,
     get_endpoints,
 )
 from mlflow.store.entities.paged_list import PagedList
 from mlflow.store.model_registry import (
@@ -799,5 +800,39 @@ def test_delete_artifact_mlflow_artifacts_throws_for_malicious_path(enable_serve
     response = _delete_artifact_mlflow_artifacts(path)
     assert response.status_code == 400
     json_response = json.loads(response.get_data())
     assert json_response["error_code"] == ErrorCode.Name(INVALID_PARAMETER_VALUE)
     assert json_response["message"] == "Invalid path"
+
+
+def test_local_file_read_write_by_pass_vulnerability():
+    request = mock.MagicMock()
+    request.method = "POST"
+    request.content_type = "application/json; charset=utf-8"
+    request.get_json = mock.MagicMock()
+    request.get_json.return_value = {
+        "name": "hello",
+        "artifact_location": "http://host#/abc/etc/",
+    }
+    msg = _get_request_message(CreateExperiment(), flask_request=request)
+    with mock.patch("mlflow.server.handlers._get_request_message", return_value=msg):
+        response = _create_experiment()
+        json_response = json.loads(response.get_data())
+        assert json_response["message"] == "'artifact_location' URL can't include fragment part."
+
+    # Test if source is a local filesystem path, `_validate_source` validates that the run
+    # artifact_uri is also a local filesystem path.
+    run_id = uuid.uuid4().hex
+    with mock.patch("mlflow.server.handlers._get_tracking_store") as mock_get_tracking_store:
+        mock_get_tracking_store().get_run(
+            run_id
+        ).info.artifact_uri = f"http://host/{run_id}/artifacts/abc"
+
+        with pytest.raises(
+            MlflowException,
+            match=(
+                "the run_id request parameter has to be specified and the local "
+                "path has to be contained within the artifact directory of the "
+                "run specified by the run_id"
+            ),
+        ):
+            _validate_source("/local/path/xyz", run_id)

Vulnerable code: 
            run_artifact_dir = pathlib.Path(local_file_uri_to_path(run.info.artifact_uri)).resolve()
            if run_artifact_dir in [source, *source.parents]:
                return
Fixed code:     if parsed_artifact_locaion.fragment:
        raise MlflowException(
            "'artifact_location' URL can't include fragment part.",
            error_code=INVALID_PARAMETER_VALUE,
        )
            if is_local_uri(run.info.artifact_uri):
                run_artifact_dir = pathlib.Path(
                    local_file_uri_to_path(run.info.artifact_uri)
                ).resolve()
                if run_artifact_dir in [source, *source.parents]:
                    return
    _validate_source,


def test_local_file_read_write_by_pass_vulnerability():
    request = mock.MagicMock()
    request.method = "POST"
    request.content_type = "application/json; charset=utf-8"
    request.get_json = mock.MagicMock()
    request.get_json.return_value = {
        "name": "hello",
        "artifact_location": "http://host#/abc/etc/",
    }
    msg = _get_request_message(CreateExperiment(), flask_request=request)
    with mock.patch("mlflow.server.handlers._get_request_message", return_value=msg):
        response = _create_experiment()
        json_response = json.loads(response.get_data())
        assert json_response["message"] == "'artifact_location' URL can't include fragment part."

    # Test if source is a local filesystem path, `_validate_source` validates that the run
    # artifact_uri is also a local filesystem path.
    run_id = uuid.uuid4().hex
    with mock.patch("mlflow.server.handlers._get_tracking_store") as mock_get_tracking_store:
        mock_get_tracking_store().get_run(
            run_id
        ).info.artifact_uri = f"http://host/{run_id}/artifacts/abc"

        with pytest.raises(
            MlflowException,
            match=(
                "the run_id request parameter has to be specified and the local "
                "path has to be contained within the artifact directory of the "
                "run specified by the run_id"
            ),
        ):
            _validate_source("/local/path/xyz", run_id)