@pmFromFile matches for empty commented line

[LeeShan87]

Hi,

We experienced a strange bug when testing libmodsecurity with crs.
The Chinese search engine ( spider/4.0(+http://www.sogou.com/docs/help/webmasters.htm#07); ) always got caught by the crs's 913100 rule which looks for suspicious user agent from a file https://github.com/SpiderLabs/owasp-modsecurity-crs/blob/v3.0/master/rules/scanners-user-agents.data

After looking in the the list of user agents we haven't found anything close which could trigger this rule.

With PostMan I played with this user agent string and found that the problematic character was: "#"
spider/4.0(+http://www.sogou.com/docs/help/webmasters.htm#07); -> trigger 913100
spider/4.0(+http://www.sogou.com/docs/help/webmasters.htm07); -> not trigger 913100

After removing every empty comment line and reloading ModSecurity it worked as it accepted.

Empty and commented lines should be ignored as the reference manual says: https://github.com/SpiderLabs/ModSecurity/wiki/Reference-Manual#pmFromFile

[dune73]

I can confirm this issue. Surfaced with CRS.

See SpiderLabs/owasp-modsecurity-crs#1215

[lifeforms]

We have included a workaround for this problem in CRS 3 development branch by removing lonely # lines in pmf data files.

SpiderLabs/owasp-modsecurity-crs#1230

We plan to issue a release candidate 3.1-RC2 around Nov 11th. Those suffering from this issue please testdrive it if you can.

For those who hit this problem in the future, upgrading your CRS to 3.1 or later will resolve the issue.

[victorhora]

@tdoubley provided a possible solution here: fb020c5

My suggestion would be having isComment() as a more "global" solution for all features that loads content from a file and needs to ignore comments (#). Might be the case for ipMatchFromFile and a few others.
isComment() should maybe reside at utils:: namespace, at string.cc.

What do you think @zimmerle ? :)

[zimmerle]

v2.x empty lines and # are ignored -
https://github.com/SpiderLabs/ModSecurity/blob/v2/master/apache2/re_operators.c#L1321

Testing fb020c5 here -
https://github.com/SpiderLabs/ModSecurity/tree/v3/dev/issue_1645

[zimmerle]

Fixed as of: 42a16c7

[zimmerle]

@tdoubley provided a possible solution here: fb020c5

My suggestion would be having isComment() as a more "global" solution for all features that loads content from a file and needs to ignore comments (#). Might be the case for ipMatchFromFile and a few others.
isComment() should maybe reside at utils:: namespace, at string.cc.

What do you think @zimmerle ? :)

Let's make it part of pmf for now, as it is already ready. Later on, we move it towards a more global thing.