Validate fragment and URL params

[Haxatron]

🛠 DevTools 🛠

Install mlflow from this PR

pip install git+https://github.com/mlflow/mlflow.git@refs/pull/10880/merge

Checkout with GitHub CLI

gh pr checkout 10880

Related Issues/PRs

#xxx

Fix an incomplete fix in https://github.com/mlflow/mlflow/pull/10653

Issue urls:

1: https://huntr.com/bounties/52a3855d-93ff-4460-ac24-9c7e4334198d/

2: https://huntr.com/bounties/dbdc6bd6-d09a-46f2-9d9c-5138a14b6e31/

What changes are proposed in this pull request?

How is this PR tested?

• [X] Existing unit/integration tests
• [X] New unit/integration tests
• [ ] Manual tests

Does this PR require documentation update?

• [X] 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?

• [X] 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.

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

Components

• [ ] area/artifacts: Artifact stores and artifact logging
• [ ] area/build: Build and test infrastructure for MLflow
• [ ] area/deployments: MLflow Deployments client APIs, server, and third-party Deployments integrations
• [ ] area/docs: MLflow documentation pages
• [ ] area/examples: Example code
• [ ] area/model-registry: Model Registry service, APIs, and the fluent client calls for Model Registry
• [ ] area/models: MLmodel format, model serialization/deserialization, flavors
• [ ] area/recipes: Recipes, Recipe APIs, Recipe configs, Recipe Templates
• [ ] area/projects: MLproject format, project running backends
• [ ] area/scoring: MLflow Model server, model deployment tools, Spark UDFs
• [X] area/server-infra: MLflow Tracking server backend
• [ ] area/tracking: Tracking Service, tracking client APIs, autologging

Interface

• [ ] area/uiux: Front-end, user experience, plotting, JavaScript, JavaScript dev server
• [ ] area/docker: Docker use across MLflow's components, such as MLflow Projects and MLflow Models
• [ ] area/sqlalchemy: Use of SQLAlchemy in the Tracking Service or Model Registry
• [ ] area/windows: Windows support

Language

• [ ] language/r: R APIs and clients
• [ ] language/java: Java APIs and clients
• [ ] language/new: Proposals for new client languages

Integrations

• [ ] integrations/azure: Azure and Azure ML integrations
• [ ] integrations/sagemaker: SageMaker integrations
• [ ] integrations/databricks: Databricks integrations

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

• [X] 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

[github-actions[bot]]

Documentation preview for a8fbbc02a1232e179a33322dc0f022b2ee143106 will be available here when this CircleCI job completes successfully.

More info

• Ignore this comment if this PR does not change the documentation.
• It takes a few minutes for the preview to be available.
• The preview is updated when a new commit is pushed to this PR.
• This comment was created by https://github.com/mlflow/mlflow/actions/runs/7665418533.

[WeichenXu123]

Thanks, but could you elaborate the case you are fixing ? :) Why it causes error in current mlflow code ?

[Haxatron]

@WeichenXu123 - The fix for this report https://huntr.com/bounties/11209efb-0f84-482f-add0-587ea6b7e850/ in https://github.com/mlflow/mlflow/pull/10653 was incomplete.

In particular it did not cover for 2 cases:

When fragment in URL - http://example.com/#/../../../../../../../../../../../../../../etc/ When params in URL - http://example.com/;..%2F..%2F..%2Fetc

[zpbrent]

Hi @WeichenXu123 and @Haxatron I have reported this bypass at the 29th Dec. 2023 in https://huntr.com/bounties/19bf02d7-6393-4a95-b9d0-d6d4d2d8c298/, not quite sure who is the first reporter and please have a check, thanks!

[Haxatron]

@harupy @WeichenXu123 can you review this?

@zpbrent. This was reported on 18 December 2023. Sorry.