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

Adding Semgrep Rules for SSRF in Java #462

Merged
merged 1 commit into from Sep 7, 2020
Merged

Conversation

@salecharohit
Copy link
Contributor

@salecharohit salecharohit commented Aug 4, 2020

Thank you for submitting a Pull Request (PR) to the Cheat Sheet Series.

🚩 If your PR is related to grammar/typo mistakes, please double-check the file for other mistakes in order to fix all the issues in the current cheat sheet.

Please make sure that for your contribution:

  • In case of a new Cheat Sheet, you have used the Cheat Sheet template.
  • All the markdown files do not raise any validation policy violation, see the policy.
  • All the markdown files follow these format rules.
  • All your assets are stored in the assets folder.
  • All the images used are in the PNG format.
  • Any references to websites have been formatted as TEXT
  • You verified/tested the effectiveness of your contribution (e.g., the defensive code proposed is really an effective remediation? Please verify it works!).
  • The CI build of your PR pass, see the build status here.

If your PR is related to an issue, please finish your PR text with the following line:

This PR covers issue #457

Thank you again for your contribution 😃

Rohit Salecha
@salecharohit salecharohit requested review from jmanico and mackowski as code owners Aug 4, 2020
@mackowski
Copy link
Collaborator

@mackowski mackowski commented Aug 5, 2020

@salecharohit thank you for PR! I will review it this week as I want to review rules also :) But it looks solid on first glance

@salecharohit
Copy link
Contributor Author

@salecharohit salecharohit commented Aug 5, 2020

@salecharohit thank you for PR! I will review it this week as I want to review rules also :) But it looks solid on first glance

Sure. I have a PR pending at Semgrep side as well. It'll be reviewed soon and will be published too.

Copy link
Collaborator

@mackowski mackowski left a comment

I reviewed the rules and it looks fine but I think that the url will change after they will merge it on sempgrep side.

[Semgrep](https://semgrep.dev/) is a command-line tool for offline static analysis. Use pre-built or custom rules to enforce code and security standards in your codebase.
Checkout the Semgrep rule for SSRF to identify/investigate for SSRF vulnerabilities in Java
[https://semgrep.dev/salecharohit:owasp_java_ssrf](https://semgrep.dev/salecharohit:owasp_java_ssrf)

This comment has been minimized.

@mackowski

mackowski Aug 11, 2020
Collaborator

I see that this link is to semgrep.dev/salecharohit:owsap_java_ssrf will it change after they merge your PR? Should we wait with this PR until they merge it master?

@mackowski
Copy link
Collaborator

@mackowski mackowski commented Aug 18, 2020

@salecharohit any updates on this? Is your PR at Semgrep side merged?

@salecharohit
Copy link
Contributor Author

@salecharohit salecharohit commented Aug 24, 2020

@salecharohit any updates on this? Is your PR at Semgrep side merged?

Yep. Here is the files in github
The Semgrep URL for testing these rules are here

Let me know if any other details are required.

@mackowski
Copy link
Collaborator

@mackowski mackowski commented Aug 31, 2020

@salecharohit apologies for late replay but I was on a short break last week :)
I think that it will be better to link to GH https://github.com/returntocorp/semgrep-rules/blob/develop/contrib/owasp/java/ssrf.yaml in line 385 what do you think?

@salecharohit
Copy link
Contributor Author

@salecharohit salecharohit commented Aug 31, 2020

@salecharohit apologies for late replay but I was on a short break last week :)
I think that it will be better to link to GH https://github.com/returntocorp/semgrep-rules/blob/develop/contrib/owasp/java/ssrf.yaml in line 385 what do you think?

Absolutely fine , please don't apologise :-)

Yep sure absolutely no problem. The only reason why I added the link to semgrep.live is because people can quickly run the rule and see the results.
Let me know your thoughts on this and I'll accordingly take action.

Update : The link mentioned by you is no longer valid. We had a re-arrangement of folders and below is the new link
https://github.com/returntocorp/semgrep-rules/blob/develop/contrib/owasp/java/ssrf/ssrf.yaml

@salecharohit
Copy link
Contributor Author

@salecharohit salecharohit commented Sep 5, 2020

Hi @mackowski awaiting your decision as my XXE rules are also ready to integrate. I'd rather wait to send a PR for XXE once the SSRF is approved.

Copy link
Collaborator

@mackowski mackowski left a comment

Looks good

@mackowski mackowski merged commit c690f59 into OWASP:master Sep 7, 2020
3 checks passed
3 checks passed
link-check
Details
lint
Details
Publishing Check
Details
@mackowski
Copy link
Collaborator

@mackowski mackowski commented Sep 7, 2020

@salecharohit go ahead and create PR for XXE - thank you for your contribution and apologies for long reply.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants