Incorrect escaping in @rx operator with macro expansion

[marcstern]

When using macro expansion with @rx operator, escaping is incorrectly applied.

Example:
SecRule ARGS "@rx localhost|%{TX.OtherHosts}"
will lead to something like SecRule ARGS "@rx localhost\|myhost1\|myhost2"
This screws up all regexes, as regex special characters will be escaped, even outside the macro-expanded varaiables.

In case I want to test against %{TX.MyHost} that isn't a regex, I can use the @contain operator.

[zimmerle]

What happens to be the version of ModSecurity in question?

[marcstern]

Sorry, 2 (trunk).
PS: It seems that I cannot tag the issue with the version number when creating it

[zimmerle]

If you fill the version number in the issue template, the automation script will tag accordingly.

[martinhsv]

Hi @marcstern ,

I'm not sure that what you are seeing is incorrect per se. When using string content like that, there isn't any way for the software to know that characters in the string are not to be treated as string literals. I.e. the code needs to make one assumption or the other.

In your case, I assume you are constructing the %{TX.OtherHosts} variable? In that case, you could perhaps consider constructing it with space delimiters and then use something like "@pm localhost %{TX.OtherHosts}"?

If that doesn't meet your needs, if you would like to describe your use case in more detail, I could probably suggest additional alternatives.

[marcstern]

Hi @martinhsv
I agree about the assumption (one way or another) but ...
If I want to treat the string as a litteral, I use the operators that are meant for literals (pm, contains, ...). If I want to treat the string as a regex, I use rx. This is simple and covers all use cases.
If rx treats the string as a litteral, I have no option for using a regex with a macro.
Work-arounds like you proposed are not possible because the real situation is obviously much more complex than the trivial example I gave (imagine TX.OtherHosts=myhost1|.*[.]mydomain for instance).

[martinhsv]

Hi @marcstern ,

I agree that there is a limitation inherent to the current assumption that is made.

However, I suspect that there are situations where the opposite assumption would also create an undesired limitation. While you have been able to utilize 'pm' and 'contains' for cases where you want special characters to be treated as literals, that may not always be true for everyone. Perhaps a TX variable needs to populated from query argument content, then combined with some complex fixed pattern and then run as a regex -- but that original query arg content needs to be treated as literal.

An additional (possibly minor) consideration is additional risk of creating undesirable regex patterns. If a regex pattern is derived from a variable that is populated from untrusted input, there is added risk of inadvertently creating opportunity for ReDoS attacks.

On the whole, I'd be reluctant to recommend that we change the current (default) rx variable-expansion functionality unless there is indication from the community that non-escaping is greatly preferred by the community in general.

If we really have a problematic gap in available functionality, a better possibility might be to create a new configuration item that controls this (possibly with a matching ctl: equivalent). Perhaps something like this SecRegexVariableExpansionEscape (with a default of 'On').

It would be interesting to assess specific use cases, though, just to make sure that there aren't reasonable workarounds already available.

[marcstern]

'SecRegexVariableExpansionEscape' is good for me (even default to off if we want to stay compatible).
Even a configure flag is OK for me.

If you can find a (generic) work-around for this, I would be happy:
SecRule ... "setvar:TX.OtherHosts=%{TX.OtherHosts}|host[.]company1[.]com"
SecRule ... "setvar:TX.OtherHosts=%{TX.OtherHosts}|.*[.]company2[.]com"
SecRule ARGS "@rx https?://(?:localhost|%{TX.OtherHosts})$" ...

[martinhsv]

I'll assume that your first two rules are conditional, such that you cannot simply put all of the options into your third rule ...

SecRule ARGS "@rx https?://(?:localhost|host\.company1\.com|.*\.company2\.com)$" ...

One alternative approach that you could consider is to structure your rules the other way around. First extract the host into a variable:

SecRule ARGS "@rx https?://(.*)$" "capture,setvar:TX.ExtractedHost=%{TX.1}...

And then (ignoring localhost for a moment) have your 1st and 2nd rules run afterwards.

SecRule TX:ExtractedHost "@Streq host.company.com" "chain ...
SecRule [whatever VARIABLES+OPERATOR you need to preserve from your original first rule] [actions formerly associated with third rule]

SecRule TX:ExtractedHost "@rx .company2.com$"" "chain ...
SecRule [whatever VARIABLES+OPERATOR you need to preserve from your original second rule] [actions formerly associated with third rule]

[marcstern]

No way, because the hostnames are added dynamically (for instance when defining a new vhost) and we don't know even their number at configuration time.
Note that, for every work-around you'll find, I'll come back with another unsupported use case ;-)
We need to have some dynamic regexes sometimes ...

[martinhsv]

Hmm. Well, I'll leave this open for the time being as an enhancement request. But without one or more fully defined use cases where the need is clear, I'm not sure how highly it will get prioritized.

[marcstern]

OK, one full use case:
I'm performing a whitelisting of URL (scheme:fqdn) that can be used inside an ARG.
Some URL are defined statically (no problems), some are dynamic.
Dynamic methods:

1. Grabbing the allowed URL from a header coming from the back-end (we're in proxy mode). In this header, I can have a fqdn, a domain or a complex regex.
2. Allowing all virtual hosts that were created (we register the name in a var dynamically through a macro).
3. Allowing only test site inside a domain (grabbed from point 1) when we are in the test environment (i.e. "test-.*%{allowedDomains}").
4. And several other ones dynamic methods, needing some kind of regex depending on their intent.