CVE-2023-1177 RFI Vulnerability in Machine Learning Lifecycle Platform, MLflow #310

hh-hunter · 2023-04-13T06:34:26Z

Hey,

this PR for the Vuln Detector Plugin for [CVE-2023-1177] (see Issue #302)

In order to verify the vulnerability, I used two different docker environments, one for the vulnerable version and the other for the fixed version

The first step is to clone my vulnerability environment warehouse to the local

git clone https://github.com/hh-hunter/ml-CVE-2023-1177
cd ml-CVE-2023-1177

Vulnerable image:

It takes several minutes to wait for the service to be accessed normally, and the exposed web service port is [15000].

docker-compose -f docker-compose-vulnerability.yml up

Safe image:

It takes several minutes to wait for the service to be accessed normally, and the exposed web service port is [15001].

docker-compose -f docker-compose-no-vulnerability.yml up

hh-hunter · 2023-04-13T06:35:38Z

@maoning hi,I have completed this plug-in, please review it as soon as possible

hh-hunter · 2023-04-24T08:05:04Z

@maoning hi，How is the audit going? What seems to be the problem?

hh-hunter · 2023-05-04T01:18:18Z

@maoning hi，How is the audit going? What seems to be the problem?

tooryx · 2023-12-28T10:20:04Z

Hi @hh-hunter,

I will be taking a look into this PR in the upcoming days.

~tooryx

Bump the version of Tsunami used by the plugin to the latest.

Add terminating newline

With recent changes, the `NetworkServiceUtils::isWebService` is more reliable and should be enough to detect if the service is a web service.

Recent changes to `NetworkServiceUtils.buildWebApplicationRootUrl` has made it more reliable and it should be sufficient to build the target URL.

Remove unused import

tooryx

Hi again @hh-hunter,

I made some changes and added some comments. Could you please take a look? And feel free to let me know if you disagree with anything.

~tooryx

tooryx · 2023-12-28T13:24:43Z

community/detectors/mlflow_cve-2023-1177/README.md

@@ -0,0 +1,17 @@
+# MLflow LFI/RFI CVE-2023-1177 Detector
+
+This detector checks for MLflow LFI/RMI  vulnerability (CVE-2023-1177).


L3: Did you mean RFI instead of RMI?

It looks like there was an issue and this is still marked as LFI/RMI.

...src/main/java/com/google/tsunami/plugins/detectors/cves/cve20231177/Cve20231177Detector.java

tooryx · 2023-12-28T13:56:08Z

...src/main/java/com/google/tsunami/plugins/detectors/cves/cve20231177/Cve20231177Detector.java

+            .replace(REPLACE_FLAG, currentModelName);
+    logger.atInfo().log("currentModelName: %s", currentModelName);
+    try {
+      HttpResponse createModeResponse =


If creating the model fails, we should not try to update it with the following request. Please consider refactoring this so that requests are only performed if the previous request has been successful.

I have added the logic to clean up the model.

Thank you for adding the cleanup function! Although it is currently called only if: adding the model succeeded AND updating it succeeded AND reading the file failed. I think it should happen in all cases as soon as the model creation succeeded.

Also, the current code is a bit difficult to read (nested if). Please consider restructuring to exit early if something goes wrong instead, for example (pseudo-code):

HttpResponse createModeResponse = httpClient.sendAsIs(...) if (createModeResponse.status().code() != 200 || !createModeResponse.bodyString().get().contains(DETECTION_STRING)) { // the model was not created, we do not need to cleanup return false; } // from this point on, the model was created, we will need cleanup for all exit paths

change vulnerability desc

change: Add cleanup logic

hh-hunter · 2024-01-02T07:16:48Z

@tooryx Please review,thk.

tooryx

Hi @hh-hunter,

I performed a second pass review. Feel free to reach out if you disagree or if you have any questions.

~tooryx

tooryx · 2024-01-03T09:48:50Z

...src/main/java/com/google/tsunami/plugins/detectors/cves/cve20231177/Cve20231177Detector.java

+                        .setPublisher("TSUNAMI_COMMUNITY")
+                        .setValue("CVE_2023_1177"))
+                .setSeverity(Severity.CRITICAL)
+                .setTitle("CVE-2023-1177 MLflow RFI")


I think you can keep both LFI/RFI here, not just RFI.

tooryx · 2024-01-03T09:53:11Z

community/detectors/mlflow_cve-2023-1177/README.md

@@ -0,0 +1,17 @@
+# MLflow LFI/RFI CVE-2023-1177 Detector
+
+This detector checks for MLflow LFI/RMI  vulnerability (CVE-2023-1177).


It looks like there was an issue and this is still marked as LFI/RMI.

tooryx · 2024-01-03T09:54:03Z

...src/main/java/com/google/tsunami/plugins/detectors/cves/cve20231177/Cve20231177Detector.java

+import static com.google.common.collect.ImmutableList.toImmutableList;
+import static com.google.common.net.HttpHeaders.CONTENT_TYPE;
+import static com.google.common.net.HttpHeaders.USER_AGENT;
+import static com.google.tsunami.common.net.http.HttpRequest.*;


Please do not use wildcard imports as they make code brittle and difficult to reason about, both for programmers and for tools.

tooryx · 2024-01-03T10:05:09Z

...src/main/java/com/google/tsunami/plugins/detectors/cves/cve20231177/Cve20231177Detector.java

+            .replace(REPLACE_FLAG, currentModelName);
+    logger.atInfo().log("currentModelName: %s", currentModelName);
+    try {
+      HttpResponse createModeResponse =


Thank you for adding the cleanup function! Although it is currently called only if: adding the model succeeded AND updating it succeeded AND reading the file failed. I think it should happen in all cases as soon as the model creation succeeded.

Also, the current code is a bit difficult to read (nested if). Please consider restructuring to exit early if something goes wrong instead, for example (pseudo-code):

HttpResponse createModeResponse = httpClient.sendAsIs(...) if (createModeResponse.status().code() != 200 || !createModeResponse.bodyString().get().contains(DETECTION_STRING)) { // the model was not created, we do not need to cleanup return false; } // from this point on, the model was created, we will need cleanup for all exit paths

hh-hunter · 2024-01-04T02:51:36Z

@tooryx Please review,thk.

Reorder import to alphabetical order

tooryx · 2024-01-04T09:34:24Z

Thank you for these changes. It looks good, I will try to get the change merged next week.

~tooryx

tooryx · 2024-01-04T13:23:14Z

Also, could you please rename the root directory of the plugin from mlflow_cve-2023-1177 to mlflow_cve_2023_1177 as hyphens are not accepted in plugin directories. This requires changing the settings.gradle file as well. I do not have direct access to push on your forked branch to perform the change myself.

…lugin/add_cve_2023_1177

hh-hunter · 2024-01-05T01:25:30Z

Okay, can you check my other submitted issues? It has been a very long time since the last one.

add cve-2023-1177 detector plugins

7800775

maoning self-assigned this Dec 5, 2023

tooryx self-assigned this Dec 28, 2023

tooryx added 5 commits December 28, 2023 11:21

Update build.gradle

7780262

Bump the version of Tsunami used by the plugin to the latest.

Update settings.gradle

c58bfb5

Add terminating newline

Remove custom web service detection

9cf5202

With recent changes, the `NetworkServiceUtils::isWebService` is more reliable and should be enough to detect if the service is a web service.

Remove custom URI creation logic

0e66c18

Recent changes to `NetworkServiceUtils.buildWebApplicationRootUrl` has made it more reliable and it should be sufficient to build the target URL.

Update Cve20231177Detector.java

02ad6c5

Remove unused import

tooryx reviewed Dec 28, 2023

View reviewed changes

tooryx mentioned this pull request Dec 28, 2023

Fix the docker compose instructions hh-hunter/ml-CVE-2023-1177#1

Merged

hh-hunter added 4 commits January 2, 2024 13:41

Update README.md

10e6b27

change vulnerability desc

add clean model logic

f98cac0

change: Add cleanup logic

224157c

Update Cve20231177Detector.java

646e56b

change: Add cleanup logic

tooryx reviewed Jan 3, 2024

View reviewed changes

hh-hunter added 2 commits January 4, 2024 10:44

change: change vuln name and optimize code

32044a0

merge

f5a32b4

Update Cve20231177Detector.java

fbf63c7

Reorder import to alphabetical order

hh-hunter added 2 commits January 5, 2024 09:24

change model name

e0ebe82

Merge remote-tracking branch 'origin/plugin/add_cve_2023_1177' into p…

029604c

…lugin/add_cve_2023_1177

rename directory name

abdf058

rename directory name

5097c91

copybara-service bot merged commit 10a2d03 into google:master Jan 8, 2024

		@@ -0,0 +1,17 @@
		# MLflow LFI/RFI CVE-2023-1177 Detector

		This detector checks for MLflow LFI/RMI vulnerability (CVE-2023-1177).

CVE-2023-1177 RFI Vulnerability in Machine Learning Lifecycle Platform, MLflow #310

CVE-2023-1177 RFI Vulnerability in Machine Learning Lifecycle Platform, MLflow #310

Uh oh!

Conversation

hh-hunter commented Apr 13, 2023

Uh oh!

hh-hunter commented Apr 13, 2023

Uh oh!

hh-hunter commented Apr 24, 2023

Uh oh!

hh-hunter commented May 4, 2023

Uh oh!

tooryx commented Dec 28, 2023

Uh oh!

tooryx left a comment

Choose a reason for hiding this comment

Uh oh!

tooryx Dec 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hh-hunter commented Jan 2, 2024

Uh oh!

tooryx left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hh-hunter commented Jan 4, 2024

Uh oh!

tooryx commented Jan 4, 2024

Uh oh!

tooryx commented Jan 4, 2024

Uh oh!

hh-hunter commented Jan 5, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tooryx Dec 28, 2023 •

edited

Loading