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

[feature] Harden maven verifier plugin against command injections #665

Open
AdamKorcz opened this issue Jul 20, 2023 · 4 comments
Open

[feature] Harden maven verifier plugin against command injections #665

AdamKorcz opened this issue Jul 20, 2023 · 4 comments
Assignees
Labels
area:hardening Issue related to security hardening area:maven An issue with the maven builder type:feature New feature request

Comments

@AdamKorcz
Copy link
Contributor

Is your feature request related to a problem? Please describe.
The Maven slsa verifier plugin (slsa-framework/slsa-github-generator#2380) has a potential command injection when invoking the verifier.

Describe the solution you'd like
The plugin should be audited and hardened against command injections.

@laurentsimon
Copy link
Contributor

Transferring this issue to the slsa-verifier repo

@laurentsimon laurentsimon transferred this issue from slsa-framework/slsa-github-generator Jul 21, 2023
@ianlewis ianlewis added area:hardening Issue related to security hardening area:maven An issue with the maven builder labels Jul 24, 2023
@ianlewis
Copy link
Member

It seems like the command arguments are split in a semi-sane way which is not immediately obvious to me that it's vulnerable to an attack though it would be good if we could use an alternative way to execute the command that allows you to specify the command args as a list of strings.

https://github.com/sonatype/plexus-utils/blob/5ba6cfcca911200b5b9d2b313bb939e6d7cbbac6/src/main/java/org/codehaus/plexus/util/cli/CommandLineUtils.java#L302-L383

Unfortunately it feels like we would basically need to have our own implementation of mojo-executor if we actually want to harden this and not rely on the argument splitting logic in plexus-utils.

@AdamKorcz
Copy link
Contributor Author

Unfortunately it feels like we would basically need to have our own implementation of mojo-executor if we actually want to harden this and not rely on the argument splitting logic in plexus-utils.

This looks doable given its size. Alternatively we can send PRs upstream to harden it.

For now we can also limit the characters allowed in the filename passed to the verifier, but this could prevent verification of valid files that the user wants to verify - although I have never seen java dependencies with non-base64 names.

It this a blocker for the rest of the release, or can we label this verifier experimental?

@laurentsimon
Copy link
Contributor

not. blocker for the release. We can iterate on this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:hardening Issue related to security hardening area:maven An issue with the maven builder type:feature New feature request
Projects
None yet
Development

No branches or pull requests

3 participants