search

SpiderLabs / owasp-modsecurity-crs

OWASP ModSecurity Core Rule Set (CRS) Project (Official Repository)
https://modsecurity.org/crs
Apache License 2.0
2.45k stars 726 forks source link

Rule 921120: False positive #1615

Open Taiki-San opened 4 years ago

Taiki-San commented 4 years ago

Type of Issue

False positive.

Description

The regex doesn't check if there is a payload after fairly common keywords. We had a false positive on the pattern ...\r\n\r\nlocation:\r\n.... This specific case could have avoided if the regex was followed by something like \s+\w+. What do you think?

Confirmation

[X] I have removed any personal data (email addresses, IP addresses, passwords, domain names) from any logs posted.

franbuehler commented 4 years ago

Yes, I think this issue could be solved by adding a \s\w+ maybe even a \s+[\w.:/]+ after location: so that all sorts of valid location headers match.

But, as far as I know empty headers are valid too, but they are ignored by the browser. So we potentially add an evasion here.

That's why I would only add my suggested regex to the location header and not all of the headers in this rule. Original rule regex: [\r\n]\W*?(?:content-(?:type|length)|set-cookie|location):

Other opinions?

dune73 commented 4 years ago

No, I can not see where an empty contnt-type, content-length, set-cookie or location header can be considered standard behaviour. Sure, the protocol does allow it, but that does not mean that it happens in real life.

franbuehler commented 4 years ago

So I add my suggested regex to all headers?

dune73 commented 4 years ago

I think that should work, yes.

franbuehler commented 4 years ago

Ok, I'll do that. Thanks for your fast feedback!

dune73 commented 4 years ago

Decision during the CRS project chat on March 2, 2020: @franbuehler will solve this.

https://github.com/SpiderLabs/owasp-modsecurity-crs/issues/1683#issuecomment-593584538