CVE-2023-2780: Path Traversal: '\..\filename' (CWE-29)
value: Path Traversal: '\..\filename' 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 '\..\filename' (leading backslash dot dot) sequences that can resolve to a location that is outside of that directory.
CVSS v3 base score: 9.8
Published: 2023-05-17T21:15Z
Repository: https://github.com/mlflow/mlflow
Language: Python
Files changed: mlflow/environment_variables.py, mlflow/server/handlers.py, mlflow/utils/uri.py, tests/tracking/integration_test_utils.py, tests/tracking/test_rest_tracking.py, tests/utils/test_uri.py
Commit date: 2023-03-31 08:16:10 +0000
Commit message: Disallow using a file URI as model version source (#8126)

* Add a test

Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>

* Add a test

Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>

* Disallow file URI

Signed-off-by: harupy <hkawamura0130@gmail.com>

* Add a test

Signed-off-by: harupy <hkawamura0130@gmail.com>

* Ensure artifact URI is a file URI

Signed-off-by: harupy <hkawamura0130@gmail.com>

* Add a new test

Signed-off-by: harupy <hkawamura0130@gmail.com>

* Fix doc comment

Signed-off-by: harupy <hkawamura0130@gmail.com>

* Add test case

Signed-off-by: harupy <hkawamura0130@gmail.com>

* Fix is_local_uri

Signed-off-by: harupy <hkawamura0130@gmail.com>

* Fix valiation logic

Signed-off-by: harupy <hkawamura0130@gmail.com>

* Include 127.0.0.1

Signed-off-by: harupy <hkawamura0130@gmail.com>

* Update comment

Signed-off-by: harupy <hkawamura0130@gmail.com>

* Remove dot

Signed-off-by: harupy <hkawamura0130@gmail.com>

* Update mlflow/environment_variables.py

Signed-off-by: Harutaka Kawamura <hkawamura0130@gmail.com>

---------

Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>
Signed-off-by: harupy <hkawamura0130@gmail.com>
Signed-off-by: Harutaka Kawamura <hkawamura0130@gmail.com>
Diff: diff --git a/mlflow/environment_variables.py b/mlflow/environment_variables.py
index e6df8b7e6..1b06cf77e 100644
--- a/mlflow/environment_variables.py
+++ b/mlflow/environment_variables.py
@@ -200,5 +200,13 @@ MLFLOW_ARTIFACT_UPLOAD_DOWNLOAD_TIMEOUT = _EnvironmentVariable(
 #: setting this environment variable to be ``cpu``. Currently, this
 #: variable is only supported for the MLflow PyTorch flavor.
 MLFLOW_DEFAULT_PREDICTION_DEVICE = _EnvironmentVariable(
     "MLFLOW_DEFAULT_PREDICTION_DEVICE", str, None
 )
+
+#: Specifies whether or not to allow using a file URI as a model version source.
+#: Please be aware that setting this environment variable to True is potentially risky
+#: because it can allow access to arbitrary files on the specified filesystem
+#: (default: ``False``).
+MLFLOW_ALLOW_FILE_URI_AS_MODEL_VERSION_SOURCE = _BooleanEnvironmentVariable(
+    "MLFLOW_ALLOW_FILE_URI_AS_MODEL_VERSION_SOURCE", False
+)
diff --git a/mlflow/server/handlers.py b/mlflow/server/handlers.py
index 03cda4e9f..01e5d8c23 100644
--- a/mlflow/server/handlers.py
+++ b/mlflow/server/handlers.py
@@ -82,13 +82,14 @@ from mlflow.tracking._model_registry.registry import ModelRegistryStoreRegistry
 from mlflow.tracking._tracking_service.registry import TrackingStoreRegistry
 from mlflow.utils.mime_type_utils import _guess_mime_type
 from mlflow.utils.proto_json_utils import message_to_json, parse_dict
 from mlflow.utils.validation import _validate_batch_log_api_req
 from mlflow.utils.string_utils import is_string_type
-from mlflow.utils.uri import is_local_uri
+from mlflow.utils.uri import is_local_uri, is_file_uri
 from mlflow.utils.file_utils import local_file_uri_to_path
 from mlflow.tracking.registry import UnsupportedModelRegistryStoreURIException
+from mlflow.environment_variables import MLFLOW_ALLOW_FILE_URI_AS_MODEL_VERSION_SOURCE
 
 _logger = logging.getLogger(__name__)
 _tracking_store = None
 _model_registry_store = None
 _artifact_repo = None
@@ -1320,27 +1321,37 @@ def _delete_registered_model_tag():
     )
     return _wrap_response(DeleteRegisteredModelTag.Response())
 
 
 def _validate_source(source: str, run_id: str) -> None:
-    if not is_local_uri(source):
-        return
-
-    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
-
-    raise MlflowException(
-        f"Invalid source: '{source}'. To use a local path as 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.",
-        INVALID_PARAMETER_VALUE,
-    )
+    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
+
+        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.",
+            INVALID_PARAMETER_VALUE,
+        )
+
+    # There might be file URIs that are local but can bypass the above check. To prevent this, we
+    # disallow using file URIs as model version sources by default unless it's explicitly allowed
+    # by setting the MLFLOW_ALLOW_FILE_URI_AS_MODEL_VERSION_SOURCE environment variable to True.
+    if not MLFLOW_ALLOW_FILE_URI_AS_MODEL_VERSION_SOURCE.get() and is_file_uri(source):
+        raise MlflowException(
+            f"Invalid model version source: '{source}'. MLflow tracking server doesn't allow using "
+            "a file URI as a model version source for security reasons. To disable this check, set "
+            f"the {MLFLOW_ALLOW_FILE_URI_AS_MODEL_VERSION_SOURCE.name} environment variable to "
+            "True.",
+            INVALID_PARAMETER_VALUE,
+        )
 
 
 @catch_mlflow_exception
 @_disable_if_artifacts_only
 def _create_model_version():
diff --git a/mlflow/utils/uri.py b/mlflow/utils/uri.py
index 83741d130..3353a7003 100644
--- a/mlflow/utils/uri.py
+++ b/mlflow/utils/uri.py
@@ -27,11 +27,15 @@ def is_local_uri(uri):
     if is_windows() and uri.startswith("\\\\"):
         # windows network drive path looks like: "\\<server name>\path\..."
         return False
 
     parsed_uri = urllib.parse.urlparse(uri)
-    if parsed_uri.hostname:
+    if parsed_uri.hostname and not (
+        parsed_uri.hostname == "."
+        or parsed_uri.hostname.startswith("localhost")
+        or parsed_uri.hostname.startswith("127.0.0.1")
+    ):
         return False
 
     scheme = parsed_uri.scheme
     if scheme == "" or scheme == "file":
         return True
@@ -40,10 +44,14 @@ def is_local_uri(uri):
         return True
 
     return False
 
 
+def is_file_uri(uri):
+    return urllib.parse.urlparse(uri).scheme == "file"
+
+
 def is_http_uri(uri):
     scheme = urllib.parse.urlparse(uri).scheme
     return scheme == "http" or scheme == "https"
 
 
diff --git a/tests/tracking/integration_test_utils.py b/tests/tracking/integration_test_utils.py
index 348cb74e6..59c5c1331 100644
--- a/tests/tracking/integration_test_utils.py
+++ b/tests/tracking/integration_test_utils.py
@@ -36,11 +36,11 @@ def _terminate_server(process, timeout=10):
     _logger.info("Terminating server...")
     process.terminate()
     process.wait(timeout=timeout)
 
 
-def _init_server(backend_uri, root_artifact_uri):
+def _init_server(backend_uri, root_artifact_uri, extra_env=None):
     """
     Launch a new REST server using the tracking store specified by backend_uri and root artifact
     directory specified by root_artifact_uri.
     :returns A tuple (url, process) containing the string URL of the server and a handle to the
              server process (a multiprocessing.Process object).
@@ -55,10 +55,11 @@ def _init_server(backend_uri, root_artifact_uri):
         ],
         env={
             **os.environ,
             BACKEND_STORE_URI_ENV_VAR: backend_uri,
             ARTIFACT_ROOT_ENV_VAR: root_artifact_uri,
+            **(extra_env or {}),
         },
     )
 
     _await_server_up_or_die(server_port)
     url = f"http://{LOCALHOST}:{server_port}"
diff --git a/tests/tracking/test_rest_tracking.py b/tests/tracking/test_rest_tracking.py
index 45ee41ece..06d7ed63e 100644
--- a/tests/tracking/test_rest_tracking.py
+++ b/tests/tracking/test_rest_tracking.py
@@ -1042,16 +1042,56 @@ def test_get_metric_history_bulk_calls_optimized_impl_when_expected(monkeypatch,
             metric_key="mock_key",
             max_results=25000,
         )
 
 
-def test_create_model_version_with_local_source(mlflow_client):
+def test_create_model_version_with_path_source(mlflow_client):
     name = "mode"
     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
+
+    # run_id is not specified
+    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://") :],
+        },
+    )
+    assert response.status_code == 400
+    assert "To use a local path as a model version" in response.json()["message"]
+
+    # run_id is specified but source is not in the run's artifact directory
+    response = requests.post(
+        f"{mlflow_client.tracking_uri}/api/2.0/mlflow/model-versions/create",
+        json={
+            "name": name,
+            "source": "/tmp",
+            "run_id": run.info.run_id,
+        },
+    )
+    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_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)
+    assert run.info.artifact_uri.startswith("file://")
     response = requests.post(
         f"{mlflow_client.tracking_uri}/api/2.0/mlflow/model-versions/create",
         json={
             "name": name,
             "source": run.info.artifact_uri,
@@ -1088,42 +1128,69 @@ def test_create_model_version_with_local_source(mlflow_client):
             "run_id": run.info.run_id,
         },
     )
     assert response.status_code == 200
 
+    # run_id is not specified
     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,
+            "source": run.info.artifact_uri,
         },
     )
-    assert response.status_code == 200
+    assert response.status_code == 400
+    assert "To use a local path as a model version" in response.json()["message"]
 
+    # run_id is specified but source is not in the run's artifact directory
     response = requests.post(
         f"{mlflow_client.tracking_uri}/api/2.0/mlflow/model-versions/create",
         json={
             "name": name,
-            "source": run.info.artifact_uri,
+            "source": "file:///tmp",
         },
     )
     assert response.status_code == 400
-    resp = response.json()
-    assert "Invalid source" in resp["message"]
+    assert "To use a local path as a model version" in response.json()["message"]
 
     response = requests.post(
         f"{mlflow_client.tracking_uri}/api/2.0/mlflow/model-versions/create",
         json={
             "name": name,
-            "source": "/tmp",
+            "source": "file://123.456.789.123/path/to/source",
             "run_id": run.info.run_id,
         },
     )
     assert response.status_code == 400
-    resp = response.json()
-    assert "Invalid source" in resp["message"]
+    assert "MLflow tracking server doesn't allow" in response.json()["message"]
+
+
+def test_create_model_version_with_file_uri_env_var(tmp_path):
+    backend_uri = tmp_path.joinpath("file").as_uri()
+    url, process = _init_server(
+        backend_uri,
+        root_artifact_uri=tmp_path.as_uri(),
+        extra_env={"MLFLOW_ALLOW_FILE_URI_AS_MODEL_VERSION_SOURCE": "true"},
+    )
+    try:
+        mlflow_client = MlflowClient(url)
+
+        name = "test"
+        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": "file://123.456.789.123/path/to/source",
+                "run_id": run.info.run_id,
+            },
+        )
+        assert response.status_code == 200
+    finally:
+        _terminate_server(process)
 
 
 def test_logging_model_with_local_artifact_uri(mlflow_client):
     from sklearn.linear_model import LogisticRegression
 
diff --git a/tests/utils/test_uri.py b/tests/utils/test_uri.py
index df2171657..ab5eaff87 100644
--- a/tests/utils/test_uri.py
+++ b/tests/utils/test_uri.py
@@ -89,10 +89,15 @@ def test_get_db_info_from_uri_errors_invalid_profile(server_uri):
 def test_is_local_uri():
     assert is_local_uri("mlruns")
     assert is_local_uri("./mlruns")
     assert is_local_uri("file:///foo/mlruns")
     assert is_local_uri("file:foo/mlruns")
+    assert is_local_uri("file://./mlruns")
+    assert is_local_uri("file://localhost/mlruns")
+    assert is_local_uri("file://localhost:5000/mlruns")
+    assert is_local_uri("file://127.0.0.1/mlruns")
+    assert is_local_uri("file://127.0.0.1:5000/mlruns")
 
     assert not is_local_uri("file://myhostname/path/to/file")
     assert not is_local_uri("https://whatever")
     assert not is_local_uri("http://whatever")
     assert not is_local_uri("databricks")

Vulnerable code: from mlflow.utils.uri import is_local_uri
    if not is_local_uri(source):
        return

    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

    raise MlflowException(
        f"Invalid source: '{source}'. To use a local path as 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.",
        INVALID_PARAMETER_VALUE,
    )
    if parsed_uri.hostname:
def _init_server(backend_uri, root_artifact_uri):
def test_create_model_version_with_local_source(mlflow_client):
            "source": run.info.artifact_uri[len("file://") :],
            "run_id": run.info.run_id,
    assert response.status_code == 200
            "source": run.info.artifact_uri,
    resp = response.json()
    assert "Invalid source" in resp["message"]
            "source": "/tmp",
    resp = response.json()
    assert "Invalid source" in resp["message"]
Fixed code: 
#: Specifies whether or not to allow using a file URI as a model version source.
#: Please be aware that setting this environment variable to True is potentially risky
#: because it can allow access to arbitrary files on the specified filesystem
#: (default: ``False``).
MLFLOW_ALLOW_FILE_URI_AS_MODEL_VERSION_SOURCE = _BooleanEnvironmentVariable(
    "MLFLOW_ALLOW_FILE_URI_AS_MODEL_VERSION_SOURCE", False
)
from mlflow.utils.uri import is_local_uri, is_file_uri
from mlflow.environment_variables import MLFLOW_ALLOW_FILE_URI_AS_MODEL_VERSION_SOURCE
    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

        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.",
            INVALID_PARAMETER_VALUE,
        )

    # There might be file URIs that are local but can bypass the above check. To prevent this, we
    # disallow using file URIs as model version sources by default unless it's explicitly allowed
    # by setting the MLFLOW_ALLOW_FILE_URI_AS_MODEL_VERSION_SOURCE environment variable to True.
    if not MLFLOW_ALLOW_FILE_URI_AS_MODEL_VERSION_SOURCE.get() and is_file_uri(source):
        raise MlflowException(
            f"Invalid model version source: '{source}'. MLflow tracking server doesn't allow using "
            "a file URI as a model version source for security reasons. To disable this check, set "
            f"the {MLFLOW_ALLOW_FILE_URI_AS_MODEL_VERSION_SOURCE.name} environment variable to "
            "True.",
            INVALID_PARAMETER_VALUE,
        )
    if parsed_uri.hostname and not (
        parsed_uri.hostname == "."
        or parsed_uri.hostname.startswith("localhost")
        or parsed_uri.hostname.startswith("127.0.0.1")
    ):
def is_file_uri(uri):
    return urllib.parse.urlparse(uri).scheme == "file"


def _init_server(backend_uri, root_artifact_uri, extra_env=None):
            **(extra_env or {}),
def test_create_model_version_with_path_source(mlflow_client):
    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

    # run_id is not specified
    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://") :],
        },
    )
    assert response.status_code == 400
    assert "To use a local path as a model version" in response.json()["message"]

    # run_id is specified but source is not in the run's artifact directory
    response = requests.post(
        f"{mlflow_client.tracking_uri}/api/2.0/mlflow/model-versions/create",
        json={
            "name": name,
            "source": "/tmp",
            "run_id": run.info.run_id,
        },
    )
    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_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)
    assert run.info.artifact_uri.startswith("file://")
    # run_id is not specified
            "source": run.info.artifact_uri,
    assert response.status_code == 400
    assert "To use a local path as a model version" in response.json()["message"]
    # run_id is specified but source is not in the run's artifact directory
            "source": "file:///tmp",
    assert "To use a local path as a model version" in response.json()["message"]
            "source": "file://123.456.789.123/path/to/source",
    assert "MLflow tracking server doesn't allow" in response.json()["message"]


def test_create_model_version_with_file_uri_env_var(tmp_path):
    backend_uri = tmp_path.joinpath("file").as_uri()
    url, process = _init_server(
        backend_uri,
        root_artifact_uri=tmp_path.as_uri(),
        extra_env={"MLFLOW_ALLOW_FILE_URI_AS_MODEL_VERSION_SOURCE": "true"},
    )
    try:
        mlflow_client = MlflowClient(url)

        name = "test"
        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": "file://123.456.789.123/path/to/source",
                "run_id": run.info.run_id,
            },
        )
        assert response.status_code == 200
    finally:
        _terminate_server(process)
    assert is_local_uri("file://./mlruns")
    assert is_local_uri("file://localhost/mlruns")
    assert is_local_uri("file://localhost:5000/mlruns")
    assert is_local_uri("file://127.0.0.1/mlruns")
    assert is_local_uri("file://127.0.0.1:5000/mlruns")