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

RCE through TorchServe Management API #360

Closed
wants to merge 14 commits into from

Conversation

execveat
Copy link

This plugin detects exposed TorchServe Management API instances, assessing the remote code execution (RCE) risk. This risk can be caused by ShellTorch/CVE-2022-1471 (insecure defaults), or by server misconfiguration.

The exploitation chain used is different from the one described by Oligo Security (https://www.oligo.security/shelltorch) and does not rely on insecure deserialization, so it achieves command execution on all tested TorchServe versions, including the latest one (9.0).

The exploitation occurs through adding a malicious model, so there is need for staging which AFAIK is not supported by the Callback Server right now, so there are four modes to achieve the best results out of box & provide additional customization options:

  • STATIC: Users can manually host the malicious model and provide its URL to the plugin.
  • LOCAL: The plugin autonomously spawns a web server to host the model, requiring the host and port for binding and an externally reachable URL.
  • SSRF: Utilizes the Tsunami callback server's URL as the model's path. Any callback is considered a successful verification.
  • BASIC: Performs basic fingerprinting of the TorchServe Management API.

Configuration supports both CLI args and config file. If no configuration has been provided, plugin falls back to SSRF if callback server has been enabled or BASIC if it has been not. In the BASIC mode the severity of the finding is set to LOW, in other modes it's CRITICAL - but only if the verification has been confirmed, otherwise there is no finding at all.

@tooryx tooryx added the Contributor main The main issue a contributor is working on (top of the contribution queue). label Feb 1, 2024
@tooryx tooryx linked an issue Feb 2, 2024 that may be closed by this pull request
@maoning maoning self-assigned this Feb 6, 2024
@nttran8 nttran8 assigned nttran8 and unassigned maoning Feb 27, 2024
@nttran8
Copy link
Collaborator

nttran8 commented Jun 11, 2024

Hi @execveat! Unfortunately, I don't have access to update any of the submitted files, and we are seeing linter errors on our end. Please read #68 for how the file structure and linter should be formatted. For example, all codes should be placed under the community folder and replace import * to individual class/function import.

Just curious if there's a need for empty file serialized.pt. Please also clean up the commented out codes from TorchServeManagementApiTestBase.java and update all 2023 references to 2024. Thank you :)

@nttran8 nttran8 assigned execveat and unassigned nttran8 Jun 12, 2024
@execveat
Copy link
Author

execveat commented Jul 5, 2024

Hey, @nttran8! I've expanded the wildcard imports, and I don't see any other linter issues right now. Here is how I'm runninggoogle-java-formatter, please advise if it requires any additional customizations:

find doyensec/ -name '*.java' -exec java -jar ../google-java-format-1.22.0-all-deps.jar -i {} \;

Report publisher DOYENSEC and the ./doyensec/ directory are intentional (as opposed to TSUNAMI_COMMUNITY and ./community/), as AFAIK that's the intended place for the plugins contributed by Doyensec. Is this wrong? The doyensec directory is already present in the Google's tree (https://github.com/google/tsunami-security-scanner-plugins/tree/master/doyensec).

I don't mind renaming these back to TSUNAMI_COMMUNITY / ./community/, just want to make sure we have clarity on these IDs going forward.

The empty serialized.pt file is intentional. That whole resources directory is used to build a malicious TorchServe model, and the empty file is referenced in the mode.serializedFile field of the model's manifest: https://github.com/google/tsunami-security-scanner-plugins/pull/360/files#diff-d6e8541970628da8ffd5a41003ad382e503779af7e0508933ce508dc191b359cR5.

The model is a ZIP file, so it could have been provided as a binary as well, however I felt that building the archive dynamically is a better approach for transparency reasons, given it is demonstrating remote code execution.

Copy link
Member

@tooryx tooryx left a comment

Choose a reason for hiding this comment

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

First round of code review: so far, so good. I will test it more thoroughly soon. Could you address the few notes?

Thanks
~tooryx

}

ext {
tsunamiVersion = '0.0.14'
Copy link
Member

Choose a reason for hiding this comment

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

Please, ensure to use the latest version here.

}

@Test
public void isServiceVulnerable_returnsNullIfServiceIsNotTorchServe() throws IOException {
Copy link
Member

Choose a reason for hiding this comment

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

Please consider rewriting the tests in the format fuction_condition_result, for example here: isServiceVulnerable_whenBasicAndVulnerable_returnsNull

@leonardo-doyensec
Copy link
Collaborator

@tooryx we have moved the PR here #557 due to CLA issues

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Contributor main The main issue a contributor is working on (top of the contribution queue).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PRP: TorchServe Management API - Remote Code Execution
5 participants