Coreruleset: 932100 rule triggered with html decimal entities because of using legacy ISO-8859 charsets family

[piotrpazola]

Description

PROBLEM:
HTML decimal entity is blocked if the entity is before the dot sign and space or new line sign. I.e. the value of someargname: Something „The Title”. After space or new line more characters

AFFECTED CHARSETS OF ISO-8859 group:
ISO-8859-2, ISO-8859-3, ISO-8859-4, ISO-8859-5, ISO-8859-6, ISO-8859-7, ISO-8859-8, ISO-8859-10, ISO-8859-14, ISO-8859-15 are affected because the chars are encoded as decimal html „ and ” entities:
someargname=Something+%26%238222%3BThe+Title%26%238221%3B.+After+space+or+new+line+more+characters

NOT AFFECTED CHARSETS OF ISO-8859 group:
1) ISO-8859-1, ISO-8859-9 are not affected because the chars are encoded as unicode hex 84 control and hex 94 cancel chars:
someargname=Something+%84The+Title%94.+After+space+or+new+line+more+characters

2) ISO-8859-13 is not affected
someargname=Something+%A5The+Title%A1.+After+space+or+new+line+more+characters

3) ISO-8859-16 is not affected
someargname=Something+%A5The+Title%B5.+After+space+or+new+line+more+characters

4) ISO-8859-11 could be affected, the first sign encoded as decimal html „ entity, the second one is encoded as unicode hex 94 cancel char. If the dot and space or new line lays after the first sign, the false positive will appear:
someargname=Something+%26%238222%3BThe+Title%94.+After+space+or+new+line+more+characters

5) UTF-8 is not affected, the signs are encoded as hex of %E2%80%9E and %E2%80%9D, etc.

POSSIBLE SOLUTIONS:

1. I've made lots of regex tests and I've made the regex modifying template rule of 932100 with negative lookbehind 3 alternatives (lookbehind requires fixed width) before everything in the regex of the rule:

(?

to exclude 2, 3 or 4 decimal html entities

i.e.:
SecRule REQUEST_COOKIES|!REQUEST_COOKIES:/__utm/|REQUEST_COOKIES_NAMES|ARGS_NAMES|ARGS|XML:/* "@rx (?

It causes:
a) arg value of "Something „The Title”. After space or new line more characters" is passed
b) arg value of ";ifconfig" is still blocked
c) arg value of "ifconfig;. rm -rf /" is still blocked
d) arg value of "ifconfig;. rm -rf /; Something „The Title”. After space or new line more characters" is blocked
e) arg value of ";ifconfig Something „The Title”. After space or new line more characters" is blocked
f) arg value of "Something „The Title”. After space or new line more characters &&ifconfig" is blocked
etc.

1. Removing the first rule in util/regexp-assemble/regexp-932100.txt called ("Special regexp case for the '.' (source) command to prevent FP:"):

'.s.*

and modify regexp of the rule with assembled string by regexp-cmdline and regexp-assemble

• Why removing the rule is not the best option? What kind of vulnerabilities could be exploited?
• I've noticed that if I send ";ifconfig" it is passed, etc. so removing the rule of "source command" dot case seems to be not good solution at all.

THE QUESTION:
Is the first solution of negative lookbehind approach acceptable? Could anyone test it more? If yes I'll pull request the patch to REQUEST-932-APPLICATION-ATTACK-RCE.conf. It seems to be resolver of the issue and works pretty fine.

Audit Logs / Triggered Rule Numbers

2020/09/23 13:38:51 [error] 8439#0: *8406125 [client xxx] ModSecurity: Access denied with code 403 (phase 2). Matched "Operator Rx' with parameter(?:;|{||||||&|&&|n|r|$(|$((||\${|<\(|>\(|\(\s*\))\s*(?:{|\s*\(\s*|\w+=(?:[^\s]*|\$.*|\$.*|<.*|>.*|\'.*\'|\".*\")\s+|!\s*|\$)*\s*(?:'|\")*(?:[\?\*\[\]\(\)\-\|+\w'\"\./\\\\]+/)?[\\\\'\"]*(?: (5210 characters omitted)' against variableARGS:someargname' (Value: `Something „The Title”. After space or new line more characters' ) [file "owasp-modsecurity-crs-3.4.0-dev/rules/REQUEST-932-APPLICATION-ATTACK-RCE.conf"] [line "102"] [id "932100"] [rev ""] [msg "Remote Command Execution: Unix Command Injection"] [data "Matched Data: ;. After space or new line more characters found within ARGS:someargname: Something „The Title”. After space or new line more characters"] [severity "2"] [ver "OWASP_CRS/3.3.0"] [maturity "0"] [accuracy "0"] [tag "application-multi"] [tag "language-shell"] [tag "platform-unix"] [tag "attack-rce"] [tag "paranoia-level/1"] [tag "OWASP_CRS"] [tag "capec/1000/152/248/88"] [tag "PCI/6.5.2"] [hostname "xxx"] [uri "/"] [unique_id "537e23be1f5e9e4912c83747da0d0ff5"] [ref "o32,42v956,74"], client: xxx, server: xxx, request: "POST / HTTP/2.0", host: "xxx", referrer: "https://xxx/"

Your Environment

• CRS version: 3.4.0-dev
• Paranoia level setting: 1
• ModSecurity version (e.g., 2.9.3): 3.0.3
• Web Server and version: nginx 1.18.0
• Operating System and version: Debian 9

Confirmation

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

[franbuehler]

I try to understand this problem and explain it here, also for myself ;-) Sorry if it is already clear to everyone else.

From: „The Title”. After space

Minimal string that matches: ”. A
Because for special charsets this string seems to be encoded as %26%238221%3B.+A which is: ”.+A

And ”.+A triggers the rule 932100:

Minimal string that triggers this rule is: ;. A

And now @piotrpazola adds a negative lookbehind so that ;.A does not trigger if a &# combination appears before this minimal string. This is smart!
Unfortunately we don't like negative lookbehinds and we want to eliminate them.

The problem of this false positive is this "command" here, that is actually not a command, but only a dot:
https://github.com/coreruleset/coreruleset/blob/v3.4/dev/util/regexp-assemble/regexp-932100.txt#L18

If we remove this line and regenerate the regexp the false positives is gone.
So option 2 of Piotr's proposals could be the better option here. In my tests ;ifconfig was still blocked.

So the questions are:

• how many false positives do we have because of these encodings? Are these fp enough to remove this dot?
• can we safely remove this dot for the source command? Or is this a big problem?

Maybe we could add this source command / dot to PL2? I would vote for this option.

Other opinions?

[dune73]

Thank you for tearing this apart @franbuehler.

So you came to the same conclusion as @piotrpazola with regards to the source command shortcut / alias in the form of the single dot on the command line?

I think the question whether there is real danger in this command to be difficult. We need more knowledge in this discussion. I'm going to place a call in the slack channel.

Generally, a tool that could help us here might be https://github.com/commixproject/commix. Also the guy behind it, or maybe twitter brings up a qualified opinion.

Then you come to a different conclusion than @piotrpazola. He thinks ;ifconfig is no longer blocked when you remove the source command, you think it is being blocked just fine. Hmm. Could it have to do with encoding?

@piotrpazola : Could you please check again and confirm if ;ifconfig passes for you?

Regardless, I think the idea to shift the rule to PL2 as a good resolution. The problem of course being, that 932100 is already split in two parts due to being overly long (since Apache 2.2 can't handle Regexes above 8K chars - or so my memory tells me). But the source shortcut command only resides in 932100, but we need to take caution since 932105 and 932106 are already existing rules. So it's all a bit complicated, but the shift would be done as follows AFAICS:

• 932100 becomes 932101
• 932101 is assigned PL2
• New rule 932100 created as a laxer sibling of 932101 but without the . source command
• 932100 is assigned PL1

[franbuehler]

So you came to the same conclusion as @piotrpazola with regards to the source command shortcut / alias in the form of the single dot on the command line?

Yes.

I think the question whether there is real danger in this command to be difficult. We need more knowledge in this discussion. I'm going to place a call in the slack channel.

Great idea. Thank you.

Then you come to a different conclusion than @piotrpazola. He thinks ;ifconfig is no longer blocked when you remove the source command, you think it is being blocked just fine.

Yes. I came to a different conclusion.

Hmm. Could it have to do with encoding?

I don't think so, because ifconfig is another line in this regex source file (https://github.com/coreruleset/coreruleset/blob/v3.4/dev/util/regexp-assemble/regexp-932100.txt#L108) and has nothing to do with the dot. But to be honest, I'm not entirely sure and would be glad to hear from @piotrpazola.

Your plan with shifting the rules seems reasonable to me.

What I can do (hopefully today):
Remove the source/dot command in the source file and run @piotrpazola regression tests.
Fortunately, he wrote so many regression tests. This is very helpful now.

[franbuehler]

I ran the regression tests with the new rule without the source command / dot.
And one true positive is indeed not detected anymore:

    desc: arg value of "ifconfig;. rm -rf /; Something „The Title”. After space or new line more characters" is blocked
    stages:
    -
      stage:
        input:
          dest_addr: 127.0.0.1
          headers:
            Host: localhost
            User-Agent: ModSecurity CRS 3 Tests
            Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.9
            Accept-Encoding: gzip, deflate, br
            Accept-Language: pl-PL,pl;q=0.9,en-US;q=0.8,en;q=0.7
            Content-Type: application/x-www-form-urlencoded
          method: POST
          port: 80
          uri: "/"
          data: "arg=ifconfig;.+rm+-rf+/;+Something+%26%238222%3BThe+Title%26%238221%3B.+After+space+or+new+line+more+characters"
          version: HTTP/1.0
        output:
          #status: 403
          log_contains: "id \"932100\""

This pattern here should match: ;. rm -rf and it doesn't match because I removed the source / dot .
I think this is what @piotrpazola meant.

This pattern is detected with the old rule... But I'm not sure if it's detected because of the false positive in the second half of the data.
If I only add the following:
data: "arg=ifconfig;.+rm+-rf+/;"
the test also fails with the old rule.

I have to check this again...

So I was wrong. The old rule correctly identified the data: "arg=ifconfig;.+rm+-rf+/;" in the regression test.

[dune73]

I really wonder whether . <command> has any exploit potential at all. But maybe I'm too dumb to see it.

[fzipi]

If this is supposedly getting to a shell, it doesn't mind if you have the dot or not. In fact, the normal behaviour is to include the . when searching for commands, unless someone removed it. So unless it is used as an evasion technique, it doesn't make much sense.

[lifeforms]

In a shell, . sources the contents of a file. For instance, if you type . blah.txt in a shell, it will execute the contents of blah.txt.

This might execute attacker code if the attacker can control the contents of that file (such as perhaps through an unrestricted upload vulnerability, image upload, etc.) That's why I originally added the . (dot space) pattern to regexp-932100.txt.

It's true that HTML entities gives us a ; often. It can be argued that the dot-space pattern is not so dangerous as the other entries, as it requires another vulnerability (unrestricted upload) to exploit. So in the case of FP, I think it's a good idea to move this pattern out of PL1 to a higher paranoia level.

[fzipi]

We might need to add declare and typeset to the list then. Otherwise you can do indirect stuff like:

[fzipi@railgun-lan coreruleset]$ declare j=.
[fzipi@railgun-lan coreruleset]$ cat test.sh 
echo "This is a test"
[fzipi@railgun-lan coreruleset]$ $j test.sh
This is a test

Or even

[fzipi@railgun-lan coreruleset]$ declare a=s
[fzipi@railgun-lan coreruleset]$ ${a}ource test.sh 
This is a test

[fzipi]

We should also add read, because of the same problem. You can read to vars and then execute.

[fzipi@railgun-lan coreruleset]$ cat read
source
[fzipi@railgun-lan coreruleset]$ read l < read
[fzipi@railgun-lan coreruleset]$ $l test.sh 
This is a test

[franbuehler]

Thank you for your opinions!

So I will propose a PR that

• removes the dot . from 932100 (laxer sibling)
• moves 932100 to 932101 (stricter sibling) at PL2

Should I also add declare, typeset and read or is this worth another PR?

[fzipi]

I think this is part of a new PR :)

[fzipi]

Also, they are very bash specific, we might want to think on different shells also? I mean, zsh has a direct tcp connection system (see man zshtcpsys)

[dune73]

Hey @piotrpazola, could you head over to PR #1954 and check if this is implemented correctly? We could then merge that and close this issue here.