Crash in msc_rules_add_file in v3.0.2

[rperper]

I originally submitted this on the user's mailing list, but was directed here.

My name is Bob Perper and I'm a developer here at LiteSpeed technologies. We include a connector for ModSecurity v3.0 in our new release of OpenLiteSpeed and have an error reported by a customer that when we reproduced it, resulted in a crash.

The customer was using the Comodo rulesset and was reporting errors like this one:
"/usr/local/lsws/conf/modsec/comodo/05_Global_Exceptions.conf failed, ret -1, reason: 'Rules error. File: /usr/local/lsws/conf/modsec/comodo/02_Global_Generic.conf. Line: 70. Column: 18. Rule id: 0 is duplicated
Rules error. File: /usr/local/lsws/conf/modsec/comodo/05_Global_Exceptions.conf. Line: 16. Column: 88. Expecting an action, got: ,t:none"'."

So we downloaded the Comodo files and tried it on our system with our connector and got similar but not exact errors. So we isolated one specific file (03_Global_Agents.conf), used it and commented out a long line rule (two lines, line 30 and 31), (file is attached). When we run openlitespeed in the debugger we call 'msc_rules_add_file' on this file, the code crashes in ModSecurity/src/rule.cc:137

So since we were skeptical about this and figured it might be a bug in OpenLiteSpeed. So we installed Open NGINX and using their connector set up a similar rule. With the exact same file, it crashed in the same call.

We tried the same action with the master branch and had the same results. Feel free to contact me directly if you have any additional questions.

Thanks,

Bob Perper
rperper@litespeedtech.com
03_Global_Agents.conf.txt

[victorhora]

Hi @rperper,

Thanks for your report! :)

Can you confirm which codebase/commit from libModSecurity you are running? There's many fixes since 3.0.2 came out and I'm wondering if that's why I can't reproduce the same issue.

Still, based on the filenames from your report, I believe you used the Comodo Rules for LiteSpeed, which as far as I know, are meant for the older mod_security LSWS module and are currently not compatible with libModSecurity.

I've got syntax issues similar as the ones you've reported until I realized that
these rules (cwaf_rules_ls) contains unsupported features including Apache-specific (and LSWS compatible) directives such as which will break libModSecurity as these are not meant to be supported inside ModSecurity.

That being said, I had none of these issues when using the Comodo Rule Set using the Nginx/ModSec_3.0 "source". So I would suggest giving it a try with those.

[rperper]

I tried with both 3.0.2 and the "master" 3.0.3 of a couple of days ago. OpenLiteSpeed just recently released support for the v3 rules and uses the same calls as Open Nginx.

Note this was a customer report and the report was a crash. I am uncomfortable with libModSecurity crashing our application and the Nginx application which is why I'm making the report. Note that the crash was triggered not with the base Comodo rule set, but the one I sent you - with the one rule commented out. And it crashed with 3.0.2 and master, not with 3.0.0. In both OpenLiteSpeed and Open Nginx.

If you need help reproducing the problem, I'll be glad to walk you through the exact procedures I used.

Thanks,

Bob

[victorhora]

Hi @rperper,

Ok, I've managed to reproduce the crash. There's a few issues here:

The first is that the rule breaking the parser due to commenting out a single rule which is part of a chained rule.

By commenting line #30 of that 03_Global_Agents.conf.txt file, the initial rule of the chain located on lines 28 and 29 can not be processed correctly as it's missing a number of mandatory parameters such as the rule ID.
If lines 28 and 29 are also commented the rules are loaded correctly by msc_rules_add_file.

The second problem leading to the crash is that due this file using CRLF, LF line terminators (as opposed to CRLF or LF) format in addition to the incorrect rule syntax is leading to the crash. If one converts the rules file to CRLF for instance, the parser will flag incorrect syntax like so:

nginx: [emerg] "modsecurity_rules_file" directive Rules error. File: 03_Global_Agents.conf.txt. Line: 13. Column: 159. Expecting an action, got: "id:210800,phase:2,pa in /usr/local/nginx/conf/nginx.conf:30,tag:'CWAF',tag:'Agents'"

I do agree that segfault is naturally not the correct behaviour for invalid syntax and/or particular line endings and ideally we should add changes to cope with this scenario, so I'm keeping the issue open and tagging accordingly until we can investigate why that particular (invalid) rule along with CRLF line endings is crashing and come up with an improved check for these cases.

That being said, I still suggest that COMODO Rules users of libModSecurity to stick with Nginx/ModSec_3.0 "source" when downloading the rules. These load fine out of the box.
As you said yourself and I experienced myself, when trying to test the cwaf_rules_ls, a lot of tuning has to be done to properly load the rules.

As mentioned, this is due cwaf_rules_ls containing unsupported features, mainly Apache scope directives such as which will not be accepted by libModSecurity as these are not meant to be supported inside ModSecurity but rather are webserver specific and might eventually be supported by the specific connector.

Again, thanks for your report. If there's anything else you would like to add for this issue, please let us know.

Thanks.

[rperper]

Yes, please don't close a problem that crashes a system! Users make mistakes and mistakes can be handled with return codes and logging. They should never be handled with application crashes. Particularly crashes that can't be intercepted by using applications. Please, please take this seriously.

Thanks,

Bob

[victorhora]

Hey @rperper, rest assured we take crashes very seriously :) We should be able to fix it before the next (3.0.3) release.

I've nailed down the issue actually being due the fact that the 03_Global_Agents.conf.txt file as is, is having a 2nd rule in a chained rule with having a disruptive action (deny, block) which shouldn't be permitted. These simple rules should reproduce the same issue:

SecRule TIME "@unconditionalMatch" \
        "id:210831,chain"

SecRule TIME "@unconditionalMatch" \
        "deny"

This should have caught those cases: https://github.com/SpiderLabs/ModSecurity/blob/v3/master/src/parser/driver.cc#L80-L105

Traces for further investigation:

Parser debug logs:

Entering state 458
Reducing stack by rule 36 (line 940):
   $1 = token "OPERATOR_UNCONDITIONAL_MATCH" (3.13-34: )
-> $$ = nterm op_before_init (3.13-34: )
Stack now 332 78 80 0
Entering state 470
Reducing stack by rule 32 (line 899):
   $1 = nterm op_before_init (3.13-34: )
-> $$ = nterm op (3.13-34: )
Stack now 332 78 80 0
Entering state 469
Reading a token: --accepting rule at line 1165 (" \
        "")
--accepting rule at line 505 ("deny")
Next token is token "Deny" (3.35-50: )
Shifting token "Deny" (3.35-50: )
Entering state 132
Reducing stack by rule 352 (line 2603):
   $1 = token "Deny" (3.35-50: )
-> $$ = nterm act (3.35-50: )
Stack now 469 332 78 80 0
Entering state 208
Reducing stack by rule 31 (line 889):
   $1 = nterm act (3.35-50: )
-> $$ = nterm actions_may_quoted (3.35-50: )
Stack now 469 332 78 80 0
Entering state 207
Reading a token: --(end of buffer or a NUL)
--accepting rule at line 638 (""
")
--(end of buffer or a NUL)
--accepting rule at line 852 ("
")
--(end of buffer or a NUL)
--EOF (start condition 0)
Next token is token "end of file" (4.1: )
Reducing stack by rule 29 (line 876):
   $1 = nterm actions_may_quoted (3.35-50: )
-> $$ = nterm actions (3.35-50: )
Stack now 469 332 78 80 0
Entering state 515
Reducing stack by rule 72 (line 1094):
   $1 = token "DIRECTIVE" (3.1-7: )
   $2 = nterm variables (3.8-12: )
   $3 = nterm op (3.13-34: )
   $4 = nterm actions (3.35-50: )
Error: popping nterm input (1.1-54: )
Stack now 0
Cleanup: discarding lookahead token "end of file" (4.1: )

GDB Backtrace:

Program received signal SIGSEGV, Segmentation fault.
modsecurity::Rule::~Rule (this=0x7bbe40, __in_chrg=<optimized out>) at rule.cc:160
160             delete m_chainedRule;
(gdb) bt
#0  modsecurity::Rule::~Rule (this=0x7bbe40, __in_chrg=<optimized out>) at rule.cc:160
#1  0x0000000000419d29 in modsecurity::Rule::~Rule (this=0x7bbe40, __in_chrg=<optimized out>) at rule.cc:162
#2  0x000000000040da9d in refCountDecreaseAndCheck (this=<optimized out>) at ../../headers/modsecurity/rule.h:85
#3  modsecurity::RulesProperties::~RulesProperties (this=0x7b5228, __in_chrg=<optimized out>) at ../../headers/modsecurity/rules_properties.h:128
#4  0x000000000044bef9 in modsecurity::Parser::Driver::~Driver (this=0x7b5220, __in_chrg=<optimized out>) at parser/driver.cc:35
#5  0x000000000044bf59 in modsecurity::Parser::Driver::~Driver (this=0x7b5220, __in_chrg=<optimized out>) at parser/driver.cc:41
#6  0x0000000000414ffe in modsecurity::Rules::loadFromUri (this=this@entry=0x7b4ba0, uri=uri@entry=0x7fffffffe640 "/home/vhora/COMODO/boom3.txt") at rules.cc:100
#7  0x000000000040abd9 in main (argc=<optimized out>, argv=<optimized out>) at rules-check.cc:80
(gdb)

[victorhora]

Seems like this association at driver.cc:83 eventually leads to the crash as it's deallocating used memory at rule.cc:160

[rperper]

Just to get a level-set on this problem, I downloaded the latest Comodo rules and tried loading the lot with NGINX. I get this error followed by a crash:
nginx: [emerg] "modsecurity_rules_file" directive Rules error. File: /home/user/space/comodo/comodo/02_Global_Generic.conf. Line: 70. Column: 18. in /usr/local/nginx
/conf/nginx.conf:57
Did you try this file with the current Comodo rule set?

[victorhora]

Hi @rperper

Sorry for the delay.

Just to get a level-set on this problem, I downloaded the latest Comodo rules and tried loading the lot with NGINX. I get this error followed by a crash:
nginx: [emerg] "modsecurity_rules_file" directive Rules error. File: /home/user/space/comodo/comodo/02_Global_Generic.conf. Line: 70. Column: 18. in /usr/local/nginx
/conf/nginx.conf:57
Did you try this file with the current Comodo rule set?

I've rechecked this again and I can not reproduce this error. "Nginx/ModSec_3.0" Comodo Rule Set loads just fine on libModSecurity.
Maybe you are still trying to load "LiteSpeed" Comodo Rules?

As mentioned before, as far as I know, some of the Comodo Rules packages are meant for older ModSecurity modules and might not be compatible with libModSecurity.

My initial investigation tells me that this issue happens due to the rule package containing unsupported web server specific (Apache-specific and LSWS compatible) directives such as <LocationMatch> which will break libModSecurity as these are not meant to be handled directly by ModSecurity anymore (rather the connector should handle it).

That being said, I had none of these issues when using the Comodo Rule Set using the Nginx/ModSec_3.0 "source". So I would suggest giving it a try with this ruleset and maybe tell Comodo to make a version of that package also for LSWS or (simply rename it?) to make it more clear for users which version they should use.

[victorhora]

As of 3517ee4 the crash should not happen anymore. As mentioned before, we had a check to avoid that specific scenario on the parser but it was not working as intended. This should be merged to master soon as part of upcoming 3.0.3 release. Thanks for the report!