Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix URI local path traversal #11473

Merged
merged 2 commits into from
Mar 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions mlflow/server/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -599,13 +599,13 @@ 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:
parsed_artifact_location = urllib.parse.urlparse(request_message.artifact_location)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix typo (locaion -> location)

if parsed_artifact_location.fragment or parsed_artifact_location.params:
raise MlflowException(
"'artifact_location' URL can't include fragment part.",
"'artifact_location' URL can't include fragments or params.",
error_code=INVALID_PARAMETER_VALUE,
)
validate_query_string(parsed_artifact_locaion.query)
validate_query_string(parsed_artifact_location.query)
experiment_id = _get_tracking_store().create_experiment(
request_message.name, request_message.artifact_location, tags
)
Expand Down
15 changes: 12 additions & 3 deletions tests/server/test_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -804,20 +804,29 @@ def test_delete_artifact_mlflow_artifacts_throws_for_malicious_path(enable_serve
assert json_response["message"] == "Invalid path"


def test_local_file_read_write_by_pass_vulnerability():
@pytest.mark.parametrize(
"uri",
[
"http://host#/abc/etc/",
"http://host/;..%2F..%2Fetc",
],
)
def test_local_file_read_write_by_pass_vulnerability(uri):
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/",
"artifact_location": uri,
}
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."
assert (
json_response["message"] == "'artifact_location' URL can't include fragments or params."
)

# Test if source is a local filesystem path, `_validate_source` validates that the run
# artifact_uri is also a local filesystem path.
Expand Down