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

Detector CVE-2021-3129 #120

Merged
merged 25 commits into from Oct 20, 2021
Merged

Detector CVE-2021-3129 #120

merged 25 commits into from Oct 20, 2021

Conversation

timoles
Copy link
Contributor

@timoles timoles commented Oct 4, 2021

Hey Google Tsunami Team,

my PR for the Vuln Detector Plugin for CVE-2021-3129 (see Issue #86)

I used the following docker images to verify the vulnerability. I should note that I made my own images as I couldn't get the official bitnami/laravel images to run without also requiring a db server for the startup. Also, the bitnami/laravel:8.4.2 docker image, which you would expect to be vulnerable to CVE-2021-3129, is not vulnerable as it builds with a patched dependency of facade/ignition. (However, any servers build before the facade/ignition patch are still vulnerable).

Vulnerable image:

docker run -p 8000:8000 timoles/laravel_cve_2021_3129_ubuntu

Safe image:

docker run -p 8000:8000 timoles/laravel_cve_2021_3129_fixed_ubuntu

Happy to hear your feedback :)

@google-cla google-cla bot added the cla: yes label Oct 4, 2021
@timoles timoles changed the title Detector CVE-2021-3129 #86 Detector CVE-2021-3129 Oct 4, 2021
Copy link
Collaborator

@magl0 magl0 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 coder review. I mostly focused on code styles and other obvious issues. Detection logic hasn't been checked yet.

@timoles
Copy link
Contributor Author

timoles commented Oct 6, 2021

First round of coder review. I mostly focused on code styles and other obvious issues. Detection logic hasn't been checked yet.

@magl0 Thanks a lot for the style suggestions. For now style wise everything should (hopefully) check out :) .

Copy link
Collaborator

@magl0 magl0 left a comment

Choose a reason for hiding this comment

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

In addition to the inline comments, can you please also:

  1. Migrate the detector code to community/detectors? Just to be consistent with the rest of the repo.
  2. Can you please also update the community/README.md file and add a link to this detector code?

@timoles
Copy link
Contributor Author

timoles commented Oct 9, 2021

Ready for another round :)

@magl0
Copy link
Collaborator

magl0 commented Oct 11, 2021

Code style looking good now. We'll start verifying the payload and tests.

@magl0
Copy link
Collaborator

magl0 commented Oct 12, 2021

This is still under code reviews and payload verification process from a third eye in the Tsunami scanner team. Once it passes we'll merge the pull request. We'll let you know if we found additional issues.

@copybara-service copybara-service bot merged commit e5a0123 into google:master Oct 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants