Re2 support 942130

[fzipi]

This is an implementation for the idea described in #2353.

These is a summary of the changes:

• splitted rule rule into chain
• set the first match to transaction variable
• checks in the chained rule using regex
• commented the meaning of 942130 data

⚠️ There was a change in the regexp, in particular, inequalities. It doesn't need to be different to obtain a logical output from the query.

[theseion]

As discussed with @fzipi, this is our plan for going forward:

1. Complete this PR for "true" tautologies (e.g. 1 = 1) and "false" tautologies (e.g. 1 != 2)
2. The "false" tautologies need a second rule (i.e., the same base rule plus an adjusted chain rule), where we use an inverted match (!@rx) instead of negative lookahead. That will allow us to convert this rule into a non-backtracking compatible rule
3. Create an additional rule for general tautology attacks at a higher paranoia level, accepting more FP's. The idea behind this is that, in general, tautology attacks don't have to come in the simple formats shown above; example: j not in [some,thing,silly]. They also don't necessarily need to produce true as their value; example SELECT * FROM x WHERE y NOT IN [], will return all records.

[fzipi]

@theseion First take on splitting this rule. Tests are failing still.

[theseion]

Looks pretty good already.

[theseion]

After digging through the source code of ModSecurity v2.9.5, we have identified a bug that effectively breaks all but the simplest use of macro expansion in @rx. ModSecurity will correctly expand the macros but will then escape the entire expression. Even though the escaping method has a _log prefix, the result of this method is then passed to PCRE, instead of the "raw" expanded expression.

ModSecurity v3 is fine, no escaping happens at all in that version.

Unfortunately, I think that means the idea is off the table for now. Even if this bug were fixed in a short amount of time, it would mean that older installations of Apache httpd would not get this rule. Unless we implement this technique "for future use" and retain fallback rules.

@dune73 What's your take?

[fzipi]

Ok, this looks like it might work.

Pushed the use of within in both rules. Do we need additional tests maybe?

[fzipi]

Added more tests. I'm sorry to say: it works like a charm!

Other than some changes in logging style, I think this is good to go!

Example:

Message: Warning. Match of "within /%{TX.2}/" against "TX:lhs_942131" required. [file "/etc/modsecurity.d/owasp-crs/rules/REQUEST-942-APPLICATION-ATTACK-SQLI.conf"] [line "636"] [id "942131"] [msg "SQL Injection Attack: SQL Boolean-based attack detected"] [data "Matched Data: 1 is not 2 found within TX:lhs_942131"] [severity "CRITICAL"] [ver "OWASP_CRS/3.4.0-dev"] [tag "modsecurity"] [tag "application-multi"] [tag "language-multi"] [tag "platform-multi"] [tag "attack-sqli"] [tag "OWASP_CRS"] [tag "capec/1000/152/248/66"] [tag "PCI/6.5.2"] [tag "paranoia-level/2"]

The match is going to be on the chained rule I guess....

[theseion]

Yay! Great stuff! I'll see whether I can review it later today.

[theseion]

Not entirely sure what the state is of this PR. Are we still assessing the URL decoding issue?

[theseion]

Not sure whether you're done yet, but there are more unresolved comments (hidden again...).

[fzipi]

Not entirely sure what the state is of this PR. Are we still assessing the URL decoding issue?

No. In the end I just removed the problematic part and kept the compatibility with the previous rule in that regard. We can take a second look after we merge this one.

[theseion]

Pretty good. Only a couple small changes left.

[fzipi]

@theseion Suggestions commited.

[theseion]

LGTM

[dune73]

Sorry I did not respond here. Overlooked it in a huge pile of mail.