Skip to content
This repository has been archived by the owner on May 14, 2020. It is now read-only.

Vulnerable regexp in rule 932140 #1354

Closed
s0md3v opened this issue Apr 15, 2019 · 13 comments
Closed

Vulnerable regexp in rule 932140 #1354

s0md3v opened this issue Apr 15, 2019 · 13 comments
Labels
PR available this issue is referenced by an active pull request

Comments

@s0md3v
Copy link

s0md3v commented Apr 15, 2019

The vulnerable regular expression is located in
/crs/rules/REQUEST-932-APPLICATION-ATTACK-RCE.conf
on line 404. [Link]

The vulnerability is caused by nested repetition operators and can be exploited with the following string

for/d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d/ %x in(x) dx
@theMiddleBlue theMiddleBlue added the PR available this issue is referenced by an active pull request label Apr 16, 2019
@theMiddleBlue
Copy link
Contributor

Unlike what you reported on #1359 I'm not able to reproduce the issue. Any help on defining a ReDoS HTTP request that matches 932140?

@spartantri
Copy link
Contributor

Hi @s0md3v ,there is a limit for following those bad regex matches in modsecurity, have you tested this in modsecurity? what is your current modsecurity setup and what is the extent of the impact you noticed?

PCRE match limit
PCRE match limit recursion

@s0md3v
Copy link
Author

s0md3v commented Apr 16, 2019

Hi @spartantri ,

I don't have modsecurity setup on my machine and I don't really need one because the vulnerability exists in the regular expression.

I simply extracted all the regular expressions from the configuration files, ran a quick grep to find the find the potentially vulnerable ones, tried to exploit them one by one by using regex101.com and then verified the impact using re.search(pattern, exploit_string) in Python.
I apologize for not using match instead of search which is the reason 4 out 5 vulnerabilities were not reproducible.

Also, I wasn't aware about the limits but I have a few questions about this implementation

  1. The timeout seems to be 1.5 seconds but what happens if the matching times out? Is the request blocked?
  2. Shouldn't we fix the root cause instead of relying on such limits? PS: I am not a regex wizard but I believe the recursion limit has nothing to with backtracking which is the real culprit.

I believe the regular expressions should be fixed but it's up to your team to make the final decision.

@spartantri
Copy link
Contributor

Unless the configuration is made purposely changed to extremely high limits this is not a high DoS vulnerability as the title suggest as the limits are like adding an {#} to the regex in place of a recursive match.

When limits are reached, depending on your particular setup and the rule logic the transaction most probably will be blocked. I agree there are some regex that need some love to make them more efficient and this fixes should definitively be implemented to prevent a possible vulnerability when an administrator sets crazy values on their setup.

Maybe we can add this to CONTRIBUTING as guidelines to keep this in mind and prevent this from happening in the future, and also tune and benchmark the CRS for performance.

@s0md3v
Copy link
Author

s0md3v commented Apr 16, 2019

Thanks for making this clear but what's the default behavior?
Does mod_security flags the request as malicious when the timeout value is reached?

@dune73
Copy link
Contributor

dune73 commented Apr 26, 2019

This issue is being referenced as CVE-2019-11388 by NIST.

I confirm that this issue is not present with ModSecurity.

I have tried to reproduce the behaviour with the following call:

curl -v "http://localhost" -d "x=for/d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d/ %x in(x) dx"

against the following setups:

  • Vanilla ModSec 2.9.3 on Apache
  • ModSec 2.9.3 on Apache with PCRE Match Limits of 50M
  • ModSec 3.0.3 on Nginx 1.3.12

None of them shows the behaviour claimed in the issue.

@s0md3v: Please explain.

@s0md3v
Copy link
Author

s0md3v commented Apr 26, 2019

I did not craft exploits for the whole strings because they could have been abused. I wrote exploits for the vulnerable sub-patterns.

@spartantri
Copy link
Contributor

@dune73 Some engines (like the one of regex101 which is different than modsec) get catastrophic backtracking errors with
for/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d/d %variable in(set) do command
But wasn't able to reproduce or had delays in modsec v2 with Apache

@s0md3v
Copy link
Author

s0md3v commented Apr 26, 2019

I tested it with Python.

@fgsch
Copy link
Contributor

fgsch commented Apr 26, 2019

From pcre:

  re> "for(?:\/[dflr].*)* %+[^ ]+ in\(.*\)\s?do"
data> for/d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d//d/ %x in(x) dx
Error -8 (match limit exceeded)

@dune73
Copy link
Contributor

dune73 commented Apr 28, 2019

Thanks for your feedback @s0md3v. I see the problem in the regex and I can confirm what @fgsch got with pcre2test. Yet I am not sure if you can really exploit this via CRS/ModSec. After all CRS rule 932140 applies two transformations to the payload and I do not see any misbehaviour of the webserver with your subpattern.

Also: Where would the subpattern and the pattern lead to different behaviours of the ModSec engine? I do not understand this completely yet.

@fgsch fgsch changed the title ReDOS Vulnerability [High] Vulnerable regexp in rule 932140 Apr 29, 2019
@fgsch
Copy link
Contributor

fgsch commented May 3, 2019

We should correct the regexp regardless since the parameter can only appear once.
I've commented on the PR, and hopefully after it's updated we can merge it if there are no objections.

@fgsch
Copy link
Contributor

fgsch commented Jul 5, 2019

Addressed in #1472

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
PR available this issue is referenced by an active pull request
Projects
None yet
Development

No branches or pull requests

5 participants