mirkodziadzka-avi
commented
4 years ago If we are looking at [0-9]+a to detect an attack - we are not extracting part of data, but only want to know if this regex would match the input.
In this case, this regex is equivalent to [0-9]a
The former will match if and only if the later will match. So we can simplify this without anchoring the request. And we would avoid the backtracking here.
So instead of
SecRule REQUEST_COOKIES|!REQUEST_COOKIES:/__utm/|REQUEST_COOKIES_NAMES|REQUEST_HEADERS:User-Agent|REQUEST_HEADERS:Referer|ARGS_NAMES|ARGS|XML:/* "@rx (?i)[\s\"'`;\/0-9=\x0B\x09\x0C\x3B\x2C\x28\x3B]+on[a-zA-Z]+[\s\x0B\x09\x0C\x3B\x2C\x28\x3B]*?=" \we could write
SecRule REQUEST_COOKIES|!REQUEST_COOKIES:/__utm/|REQUEST_COOKIES_NAMES|REQUEST_HEADERS:User-Agent|REQUEST_HEADERS:Referer|ARGS_NAMES|ARGS|XML:/* "@rx (?i)[\s\"'`;\/0-9=\x0B\x09\x0C\x3B\x2C\x28\x3B]on[a-zA-Z]+[\s\x0B\x09\x0C\x3B\x2C\x28\x3B]*?=" \
dune73
allanrbo
airween
On the Slack channel 2020-03-02, Airween pointed out some PCRE performance issues with requests containing long consecutive strings of digits. He suggested a fix using the (*SKIP) flag. Since this flag is PCRE specific, I'd like to avoid it, in order to pave the way for modsec-alternatives that use regex libraries with guarenteed linear run time (RE2, Hyperscan, and Go's Regexp, for example).
This PR relates is a suggestion to modify 941120, 942210, and 942260.
Here's my understanding of the issue: The regexes in these rules all have the option to begin matching with a repeated number of digits, such as
[0-9]+. For example the regex[0-9]+awould match11111a,111111111a,11111111111111a, etc. Now the problem arises with for example a string like11111(without ana). In this example, PCRE has consumed11111and then looks fora, doesn't find it, and therefore backtracks to try instead with just1111, then111, then11, and then finally try with1. ~To avoid this backtracking, we can instead anchor the regex before the[0-9]+, to either the beginning of the string or some character that is not a digit. The fixed example then becomes(?:^|[^0-9])[0-9]+a.~Mirko suggested below a much cleaner approach than my first approach: simply don't bother with the quantifier. Updated the PR with this cleaner approach.
Tested manually with a large request as suggested by Airween, and verified that now perform many orders of magnitude faster.
There is also a perf issue with 942130, but I will send a PR for a suggestion to fix this separately, as I have (by chance) been working on some changes to this rule for some weeks.