search

OWASP / CheatSheetSeries

The OWASP Cheat Sheet Series was created to provide a concise collection of high value information on specific application security topics.
https://cheatsheetseries.owasp.org
Creative Commons Attribution Share Alike 4.0 International
27.89k stars 3.91k forks source link

Update: Adding Semgrep Rules #457

Closed salecharohit closed 3 years ago

salecharohit commented 4 years ago

What is missing or needs to be updated?

Cheatsheet series contain plenty of code snippets which either highlight good practice or a sample vulnerable code. For Ex: This XXE code snippet very well highlights the idea of using the setFeature function whenever instantiating a DocumentBuilderFactory class. At times developers can miss out on these best practices due to many reasons Is there any way in which we can enforce these best practices or help them identify vulnerable code snippets by making using of a lightweight utility that allows them to write their own rules?

How should this be resolved?

Semgrep is an opensource tool for scanning source code supporting various languages like Java,Python,Javascript,Go,C etc ... The beauty of Semgrep is that allows the developers to write their own rules where they can enforce the security best practices or identify vulnerable code snippets. Its fast,sleek and easy to use. As an example , for the above XXE DocumentBuilderFactory code the below Semgrep rule can easily identify whether a DocumentBuilderFactory has been instantiated WITHOUT calling the setFeature function.

rules:
- id: XXE
  message: |
    Generic XXE
  metadata:
    owasp: "A4: XXE"
  severity: ERROR
  patterns:
  - pattern-either: 
    - pattern: DocumentBuilderFactory $DBF = $W.newInstance();
  - pattern-not-inside: |
      $RETURNTYPE $METHOD(...) {
        ...
        $DBF.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
        ...
      }     
  languages:
  - java

You can also try this rule live here https://semgrep.live/salecharohit:xxe

What i wish to propose is to inject Semgrep rules in the Cheatsheet series where developers can simply copy/clone/fork the rules provided at the particular topic and reuse them. For Ex: at the same XXE cheatsheet while discussing on best practices for DocumentBuilderFactory I can add a Semgrep rule or a URL of the semgrep.live editor or maybe both which the reader can then reuse in his/her code base or Semgrep rules. Please Note : Every Cheatsheet may require multiple updates.

mackowski commented 4 years ago

I like the idea (and Semgrep), we can also add Semgrep index to track and help find all Semgrep rules. I have another questions:

  1. Does Semgrep have default\good rules? I want to avoid to have rules in 2 places, if Semgrep has place with rules I would rather prefer to link to them.
  2. Do you have contact to someone from Semgrep project to help us with review?
mackowski commented 4 years ago

I see that there are some pre-written rules: https://semgrep.live/packs so I feel that it will be better to just link to this rules

salecharohit commented 4 years ago

I like the idea (and Semgrep), we can also add Semgrep index to track and help find all Semgrep rules. I have another questions:

  1. Does Semgrep have default\good rules? I want to avoid to have rules in 2 places, if Semgrep has place with rules I would rather prefer to link to them.

> Yes Semgrep has an entire public registry of rules but there maybe instances where we may require to add a few more. So I'll be adding the new rules in the Semgrep registry itself and then link it up here by referencing the code snippets provided in the cheatsheet series.

  1. Do you have contact to someone from Semgrep project to help us with review?

> Not sure if we can add them to this conversation.

salecharohit commented 4 years ago

I see that there are some pre-written rules: https://semgrep.live/packs so I feel that it will be better to just link to this rules

Yep Thats true but there maybe instances where we may require to build new rules. For Ex: https://semgrep.live/salecharohit:xxe here I've written a ruleset to identify for DocumentBuilder class whereas in the default semgrep registry they've provided only string search for XMLInputFactory. So my idea is to create Semgrep rules as per the OWASP cheatsheet series

mackowski commented 4 years ago

Hmm what about creating new pack (inside semgrep.live/packs) with rules for OWASP cheatsheet series and link to them - that way it will be easier to use and find them. I just try to avoid having 2 sets of similar content in 2 different places but I like the idea!

salecharohit commented 4 years ago

Hmm what about creating new pack (inside semgrep.live/packs) with rules for OWASP cheatsheet series and link to them - that way it will be easier to use and find them. I just try to avoid having 2 sets of similar content in 2 different places but I like the idea!

Yep that's exactly how i wanted to make. Only if i can get approval for the same I can start creating packs and make PRs Naming convention would be owasp... So for XXE owasp.java.xxe.dom.

mackowski commented 4 years ago

I like it and I will be happy to help with this work. @jmanico do you have more questions or any concerns around it?

jmanico commented 4 years ago

While SemGrep is the future of static analysis, keep in mind there are many security areas like business logic and workflows where static analysis is not effective. As long as we are not selling it as a way to discover all security issues I think it’s a great idea!

-- Jim Manico @Manicode

On Jul 29, 2020, at 7:28 AM, mackowski notifications@github.com wrote:

 I like it and I will be happy to help with this work. @jmanico do you have more questions or any concerns around it?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

salecharohit commented 4 years ago

While SemGrep is the future of static analysis, keep in mind there are many security areas like business logic and workflows where static analysis is not effective. As long as we are not selling it as a way to discover all security issues I think it’s a great idea! … -- Jim Manico @manicode On Jul 29, 2020, at 7:28 AM, mackowski @.***> wrote:  I like it and I will be happy to help with this work. @jmanico do you have more questions or any concerns around it? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

Surely. Semgrep and i guess most static analysers cannot detect Business logic issues and workflows. My idea is just to highlight some of the best ways we can write creative rules and what better context than the OWASP cheatseries.

jmanico commented 4 years ago

 I think this is a great idea, Rohit!

On 7/29/20 11:40 AM, Rohit Salecha wrote:

While SemGrep is the future of static analysis, keep in mind there
are many security areas like business logic and workflows where
static analysis is not effective. As long as we are not selling it
as a way to discover all security issues I think it’s a great idea!
… <#>
-- Jim Manico @manicode <https://github.com/manicode>
On Jul 29, 2020, at 7:28 AM, mackowski /*@*/.***> wrote:  I like
it and I will be happy to help with this work. @jmanico
<https://github.com/jmanico> do you have more questions or any
concerns around it? — You are receiving this because you were
mentioned. Reply to this email directly, view it on GitHub, or
unsubscribe.

Surely. Semgrep and i guess most static analysers cannot detect Business logic issues and workflows. My idea is just to highlight some of the best ways we can write creative rules and what better context than the OWASP cheatseries.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/OWASP/CheatSheetSeries/issues/457#issuecomment-665740112, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEBYCLOKVYNPMDGUFH5T4LR6A7F7ANCNFSM4PLJ4KXQ.

salecharohit commented 4 years ago

Shall I start working on creating the rules and send individual PRs then ?

jmanico commented 4 years ago

BRING THE HEAT ROHIT BRING THE HEAT!

That is a yes.

On 7/29/20 11:42 AM, Rohit Salecha wrote:

Shall I start working on creating the rules and send individual PRs then ?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/OWASP/CheatSheetSeries/issues/457#issuecomment-665741195, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEBYCNKREBGER4YUUV32GTR6A7M5ANCNFSM4PLJ4KXQ.

mackowski commented 4 years ago

@salecharohit I assigned this issue to you and changed status. Let us know if you need any help. Cheers!

salecharohit commented 4 years ago

Just an update for everyone that Semgrep folks have created a separate folder for all OWASP related rules. My first rule on java/ssrf has been published. So going forward please PR into this directory depending on the language you want. https://github.com/returntocorp/semgrep-rules/tree/develop/contrib/owasp

There is also a dedicated channel in Semgrep slack to discuss rules for OWASP Cheatsheet series and ASVS https://app.slack.com/client/TKABHD886/owasp

Also while speaking with Semgrep folks they introduced me to an interesting project on the same lines as what i've suggested here. Details availible here https://github.com/semgrep/rules-owasp-asvs

ben-elttam commented 4 years ago

Good work. Is there a rule pack for these new OWASP rules?

salecharohit commented 4 years ago

Good work. Is there a rule pack for these new OWASP rules?

All OWASP Cheatseries related rules wil bel aggregated here https://github.com/returntocorp/semgrep-rules/tree/develop/contrib/owasp/java

mackowski commented 3 years ago

I am closing this issue. Please feel free to create more PRs with Semgrep Rules! Thank you for your contribution @salecharohit