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

Vulnerable regexp in rule 933161 #1356

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

Vulnerable regexp in rule 933161 #1356

s0md3v opened this issue Apr 15, 2019 · 13 comments

Comments

@s0md3v
Copy link

s0md3v commented Apr 15, 2019

The vulnerable regular expression is located in /crs/rules/REQUEST-933-APPLICATION-ATTACK-PHP.conf on line 642. [Link]

The vulnerability is caused by nested repetition operators and intersecting alternate patterns. It can be exploited with the following string

next###########################################################################################
@s0md3v s0md3v changed the title ReDOS Vulnerbility [High] #2 ReDOS Vulnerbility [High] (#2) Apr 15, 2019
@s0md3v
Copy link
Author

s0md3v commented Apr 16, 2019

Hi @theMiddleBlue ,

The ReDOS vulnerabilty in #1356, #1357 and #1358 is caused by a similar sub-pattern

(?:\s|/\*.*\*/|//.*|#.*)*\(.*\)

This sub-pattern matches the following strings

         (anything)
/*anything*/(anything)
//anything(anything)
#anything(anything)

But it also matches the following

/*anything//anything#anything*/anything/anything(anything)

Is it intended behavior?
If not, this sub-pattern should be replaced with the following pattern

(?:\s*|/\*[^*]*\*/|//.*|#[^(]*)\([^*]*\)

I would like to make this clear before you agree on this fix.
Let's say we have a string #anything here, even (and) this (malicious string)
Original Pattern matches #anything here, even (and) this (malicious string)
This fix matches #anything here, even (and)

Simply, original fix goes for the longest match possible when looking for (), while the fix goes for the shortest.

@theMiddleBlue
Copy link
Contributor

If you're talking about 942330 it should be the intended behavior. Unlike what you reported on #1359 I'm not able to reproduce the issue. Any help on defining a ReDoS HTTP request that matches 942330?

@s0md3v
Copy link
Author

s0md3v commented Apr 16, 2019

@theMiddleBlue It might be due to the different regex engines we are using.
I used regex101.com to craft exploit strings and a python interpreter to confirm the exploits and determine their impact.

There are tons of sub-patterns before (?:\s|/\*.*\*/|//.*|#.*)*\(.*\) in the rule and the regex engines might be matching them against the input differently but the vulnerability in this last sub-pattern can be confirmed by the following string

############################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################################

To confirm the exploit I submitted works, check out this test
https://regex101.com/r/AlBwKV/

Let the processing finish and you will see that it is vulnerable to catastrophic backtracking.

@s0md3v
Copy link
Author

s0md3v commented Apr 16, 2019

One more thing, I searched not matched while testing so the exploit strings might not match the pattern.
The best way to confirm the vulnerabilities is to take the vulnerable sub-pattern and run a search against the exploit strings. A match can also be used given that a matching prefix is provided.

@s0md3v
Copy link
Author

s0md3v commented Apr 16, 2019

@spartantri What about this one?
Do we want to match *anything//anything#anything*anything/anything(anything)?

@spartantri
Copy link
Contributor

This one is tricky, as mentioned in the comments it aims to match different php injection evasion techniques

#   system(...)
#   system (...)
#   system\t(...)
#   system /*comment*/ (...)
#   system /*multiline \n comment*/ (...)
#   system //comment \n (...)
#   system #comment \n (...)

@s0md3v
Copy link
Author

s0md3v commented Apr 16, 2019

Well that means if the attacker entered system/*different // types of #comments in \t here*/(...)
We should stop looking for the alternatives once we detect /* and skip to the */(...) part but the currently implemented rule tries to keep looking for repetition of the different type of comments which isn't really required and causes ReDOS as discussed.
I am opening a PR to fix 3 instances of this sub-pattern.

@airween
Copy link
Contributor

airween commented Apr 16, 2019

Hi @s0md3v,

One more thing, I searched not matched while testing so the exploit strings might not match the pattern.
The best way to confirm the vulnerabilities is to take the vulnerable sub-pattern and run a search against the exploit strings. A match can also be used given that a matching prefix is provided.

just FYI, ModSecurity (ModSecurity2 and libmodsecurity3) uses pcre_exec(), which uses match criteria, instead of search.

https://github.com/SpiderLabs/ModSecurity/blob/52532a1bce0b705c0aa4365fecf727b836d37f00/apache2/msc_pcre.c#L156

https://github.com/SpiderLabs/ModSecurity/blob/b5744183866042ea9a451858a843de3c012f63ef/src/utils/regex.cc#L77-L78
https://github.com/SpiderLabs/ModSecurity/blob/b5744183866042ea9a451858a843de3c012f63ef/src/utils/regex.cc#L104-L105

I've checked the patterns you given (not all and not in both cases), but couldn't reproduce it.

I'll check them later with configured systems.

@s0md3v
Copy link
Author

s0md3v commented Apr 16, 2019

@airween
Your rule is (really complex regex)(vulnerable regex I am interested in)
I took the vulnerable sub-pattern and exploited it.
I hope that makes sense.

@s0md3v
Copy link
Author

s0md3v commented Apr 16, 2019

I will update these issues with "working" exploit strings shortly. brb

@s0md3v
Copy link
Author

s0md3v commented Apr 16, 2019

Just tried this with C++ engine and it is vulnerable to catastrophic backtracking.

Screenshot_2019-04-17_00-07-12

I am not really sure why you guys are unable to reproduce this one.
Didn't you even notice a processing delay of just 1 second?
That's strange

@dune73
Copy link
Contributor

dune73 commented Apr 26, 2019

This issue is referenced as CVE-2019-11389 by NIST.

This issues is not directly exploitable in CRS / ModSecurity.

Tested against ModSecurity 3.0.3 on Nginx 1.3.12.

curl -v "http://localhost" -d "x=next###########################################################################################"

@fgsch fgsch changed the title ReDOS Vulnerbility [High] (#2) Vulnerable regexp in rule 933161 Apr 29, 2019
@fgsch
Copy link
Contributor

fgsch commented Oct 21, 2019

Moved to #1493

@fgsch fgsch closed this as completed Oct 21, 2019
@fgsch fgsch removed the PR available this issue is referenced by an active pull request label Oct 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants