Behavior change in regex macro expansion from 2.9.7 to 2.9.8 affecting CRS 3.2.x and 3.3.x

[dune73]

Describe the bug

The way macros are expanded to regex patterns in rules changed from ModSecurity 2.9.7 to 2.9.8.

This affects CRS 3.2.x (920420, 920480) and CRS 3.3.x (920480) that both use the feature. CRS 4.x no longer uses this features and is thus not affected.

Logs and dumps

ModSecurity 2.9.7 debug log

... Resolved macro %{tx.allowed_request_content_type} to: application/x-www-form-urlencoded|multipart/form-data|text/xml|application/xml|application/soap+xml|application/x-amf|application/json|application/octet-stream|application/csp-report|application/xss-auditor-report|text/plain
... Escaping pattern [^application\/x-www-form-urlencoded|multipart\/form-data|text\/xml|application\/xml|application\/soap\+xml|application\/x-amf|application\/json|application\/octet-stream|application\/csp-report|application\/xss-auditor-report|text\/plain$]
... 
... Rule returned 0.
... No match, not chained -> mode NEXT_RULE.
...

ModSecurity 2.9.8 debug log

... Resolved macro %{tx.allowed_request_content_type} to: application/x-www-form-urlencoded|multipart/form-data|text/xml|application/xml|application/soap+xml|application/x-amf|application/json|application/octet-stream|application/csp-report|application/xss-auditor-report|text/plain
... Expanded-macro pattern [^application\/x-www-form-urlencoded|multipart\/form-data|text\/xml|application\/xml|application\/soap\+xml|application\/x-amf|application\/json|application\/octet-stream|application\/csp-report|application\/xss-auditor-report|text\/plain$]
...
... Set variable "tx.anomaly_score_pl1" to "5".
... 

To Reproduce

$ curl -H "Content-Type: application/soap+xml" -d "@example.xml" -v http://localhost

The content type application/soap+xml is in the list of allowed content-types for CRS 3.2.3 by default. With ModSecurity 2.9.8 it is suddenly no longer accepted and rule 920420 PL1 Request content type is not allowed by policy is being triggered by this Content-Type.

Expected behavior

Regex macro expansion behavior is the same between 2.9.7 and 2.9.8. The curl call above should not trigger CRS 920420 in version 3.2.3.

Server (please complete the following information):

• ModSecurity version (and connector): ModSecurity 2.9.8
• WebServer: Apache 2.4.63
• OS (and distro): Ubuntu

Rule Set (please complete the following information):

• CRS 3.2.x, CRS 3.3.x

Additional context

PR #2357 might be a candidate for the introduction of this bug.

[airween]

Hi @dune73,

thanks for reporting this.

You mentioned issue #2357, but I think the behavior was changed in #2912, where the title explains: "Do not escape special chars in regex pattern with macro".

In the "expected behavior" block you wrote:

Regex macro expansion behavior is the same between 2.9.7 and 2.9.8. The curl call above should not trigger CRS 920420 in version 3.2.3.

We can fix this only if we rolling back that change.

Anyway, I believe it's hard to decide what is the correct behavior. I think arguments from @marcstern are valid (and I guess the fix was created to solve those).

A plus argument beside this behavior that libmodsecurity3 also uses the non-escaped form (based on this comment).

So I'm curious about other users' opinion, what do they think about this.

[RedXanadu]

Adding another opinion:
A change to regular expression handling seems significant and potentially rule-breaking for many users. I'm not sure this change was sufficiently considered (or at least announced) for it to go into an x.x.7 -> x.x.8 release.

As OWASP CRS v3.3.x is currently the only release that has been supported long term it still has lots of users, as we know. If this change breaks a PL1 rule for all ModSec v2 users that seems like a big problem.

Do we know what Coraza does? Were v2 and Coraza in alignment before the change? Or are v2 and v3 and Coraza all now in alignment after the change?

[dune73]

Thank for your considered positions here.

It's indeed hard to tell what the correct behavior is. It's unfortunate this was pushed without noticing the consequences for CRS. And now a lot of time has passed and a rollback is equally discomforting. If only ModSec had a proper test suite...

What beats me is that nobody reported this before.

[airween]

I'm not sure this change was sufficiently considered (or at least announced) for it to go into an x.x.7 -> x.x.8 release.

Agree.

As OWASP CRS v3.3.x is currently the only release that has been supported long term it still has lots of users, as we know. If this change breaks a PL1 rule for all ModSec v2 users that seems like a big problem.

Yes, definitely it is.

Do we know what Coraza does? Were v2 and Coraza in alignment before the change? Or are v2 and v3 and Coraza all now in alignment after the change?

Good question - let's ask Coraza devs, so CC @fzipi, @M4tteoP, @jptosso, @jcchavezs - how Coraza handles this situation?

Also please consider the next step: if we decide to revoke that PR, then we should align libmodsecurity3 too.

[jptosso]

~~@airween and @RedXanadu

• Coraza doesn't support CRS 3.x at all
• Coraza used libmodsecurity as a reference, so we are not affected by this change~~

Edit: look at Matteo's message

[dune73]

So if I write a rule that carries a regex pattern in a variable, there is no macro expansion in Coraza (or no option to write my own rules)?

[M4tteoP]

Hey, based on the following quick try, Coraza is performing macro expansion only for some operators, not for regexes.

SecAction "id:1,phase:1,pass,setvar:'tx.var=foo|bar'"
SecRule ARGS "@rx %{TX.var}" "id:2,phase:1,deny,status:405"
SecRule ARGS "@contains %{TX.var}" "id:3,phase:1,deny,status:406"

Sending foo|bar rx does not match, contains matches:

▶ curl -v "http://localhost:8090?id=foo%7Cbar"
<HTTP/1.1 406 Not Acceptable

Sending %{TX.var}, rx matches, meaning that it is matching the values as is

▶ curl -v "http://localhost:8090?id=%25%7BTX.var%7D"
< HTTP/1.1 405 Method Not Allowed

[airween]

Thanks @M4tteoP for the explanation.

Honestly, now I'm a bit confused, because @jptosso wrote that "Coraza used libmodsecurity as a reference", and now you wrote that "Coraza is performing macro expansion only for some operators, not for regexes". But as I see libmodsecurity3 does macro expansion. (Btw the original question was that the @rx does not escape special chars - it expands the pattern, but the way has changed)

Here are the results of your test cases (used same rules):

Nginx:
curl -v "http://localhost:8080?id=foo%7Cbar"

[/?id=foo%7Cbar] [4] (Rule: 2) Executing operator "Rx" with param "foo|bar" Was: "" against ARGS.
[/?id=foo%7Cbar] [9] Target value: "foo|bar" (Variable: ARGS:id)
[/?id=foo%7Cbar] [9] Matched vars updated.
[/?id=foo%7Cbar] [4] Rule returned 1.
[/?id=foo%7Cbar] [4] (Rule: 3) Executing operator "Contains" with param "foo|bar" Was: "" against ARGS.
[/?id=foo%7Cbar] [9] Target value: "foo|bar" (Variable: ARGS:id)
[/?id=foo%7Cbar] [9] Matched vars updated.
[/?id=foo%7Cbar] [4] Rule returned 1.

Both operator matched.

curl -v "http://localhost:8080?id=%25%7BTX.var%7D"

[/?id=%25%7BTX.var%7D] [4] (Rule: 2) Executing operator "Rx" with param "foo|bar" Was: "" against ARGS.
[/?id=%25%7BTX.var%7D] [9] Target value: "%{TX.var}" (Variable: ARGS:id)
[/?id=%25%7BTX.var%7D] [4] Rule returned 0.
[/?id=%25%7BTX.var%7D] [9] Matched vars cleaned.
[/?id=%25%7BTX.var%7D] [4] (Rule: 3) Executing operator "Contains" with param "foo|bar" Was: "" against ARGS.
[/?id=%25%7BTX.var%7D] [9] Target value: "%{TX.var}" (Variable: ARGS:id)
[/?id=%25%7BTX.var%7D] [4] Rule returned 0.

None of them were matched.
Note, that as you can see in log messages: "Rx" with param "foo|bar" - this means the engine expands the macro. Same as in case of @contains.

Apache
curl -v "http://localhost:80?id=foo%7Cbar"

[/][9] Resolved macro %{TX.var} to: foo|bar
[/][6] Expanded-macro pattern [foo|bar]
[/][4] Operator completed in 233 usec.
[/][2] Warning. Pattern match "foo|bar" at ARGS:id. [file "/home/airween/src/test.conf"] [line "479"] [id "2"]
[/][4] Rule returned 1.
[/][4] Executing operator "contains" with param "%{TX.var}" against ARGS:id.
[/][9] Target value: "foo|bar"
[/][9] Resolved macro %{TX.var} to: foo|bar
[/][4] Operator completed in 9 usec.
[/][2] Warning. String match "foo|bar" at ARGS:id. [file "/home/airween/src/test.conf"] [line "480"] [id "3"]
[/][4] Rule returned 1.

curl -v "http://localhost:80?id=%25%7BTX.var%7D"

[/][9] Target value: "%{TX.var}"
[/][9] Resolved macro %{TX.var} to: foo|bar
[/][6] Expanded-macro pattern [foo|bar]
[/][4] Operator completed in 57 usec.
[/][4] Rule returned 0.
[/][4] Executing operator "contains" with param "%{TX.var}" against ARGS:id.
[/][9] Target value: "%{TX.var}"
[/][9] Resolved macro %{TX.var} to: foo|bar
[/][4] Operator completed in 4 usec.
[/][4] Rule returned 0.

As we can see here both engines follows the same way.

To solve this issue: I can imagine that we add a compilation flag (to ./configure script), which enables the old method. The default value will be disabled, but users can enable that. Later we can remove this feature, if there won't be necessary for supported CRS versions. (I just mention CRS now because nobody else reported this is an issue.) @dune73 - would be acceptable this solution for you?