Owasp-modsecurity-crs: Vlad SuperList

Created on 28 Jul 2016  Â·  22Comments  Â·  Source: SpiderLabs/owasp-modsecurity-crs

Rule 942180 evaluation:
We are first evaluating this first alternative of this rule (?:(?i:\d[\"']\s+[\"']\s+\d)
The author of the rule did not leave any indication of its use but we can guess some things from some examples.
5" " 5
6' ' 6
Etc.

This is actually a very weird oddity of SQL @attackercan. The general formula one might expect to see is the follows

SELECT * FROMxyzWHERE serialized = '$USER_INT';

It turns out the following is also valid SQL:
SELECT * FROMxyzWHERE serialized = '1''1'

But really SQL will evaluate almost anything. For instance:
SELECT * FROMxyzWHERE serialized = '1''hey'

In these weird cases, the system doesn't return anything additional, it's as if it just forgets about the extra but hey, that doesn't matter it may be used for determining if there is SQL injection.

Alright so here's the short and skinny. It seems that ANYTHING can be included after EXCEPT a number in the first digit. Now there are some false positives for valid SQL that we will incure by saying \D (like .5 and -5 are not valid)

The suggested Regex is:
(?:\d[\"']\s*?[\"']\D)

Examples:
SELECT * FROM x WHERE serialized = '1' ' 5'
~!~SELECT * FROM x WHERE serialized = '1' '5'
SELECT * FROM x WHERE serialized = '1' 'test'
SELECT * FROM x WHERE serialized = '1' ' test'
SELECT * FROM x WHERE serialized = '1' ' '
SELECT * FROM x WHERE serialized = '1' ''
SELECT * FROM x WHERE serialized = '1'''

This is likely to have HIGH false positives and therefore should be in PL 3 or 4 since 942180 is already in PL2

Stale issue

Most helpful comment

in Version 2.x rule 973300 is specifically designed to prevent known HTML tags. This is somewhat the issue with evaluating rules individually, there are other rules that are designed to detect on other XSS issues (100's as you know). As a result, this issue should not be fixed.

All 22 comments

Rule 942260 also 942250 - etc

Yeah this is weird. The not seems to have a space after it for no reason. That being said the regex in the form you described it no longer exists.

The suggestion is to remove the space after the 'not' for both rules.

Rule 921120
There is a suggestion here that a whitespace at the end of a request smuggling might be able to bypass this rule. It is interesting to note that in most cases this would fail as HTTP 1 specifies the %20 after CRLF as a header continuation. In fact in almost any situation any non-char before a header will result in a 400 Bad Request. There are some situations where this MIGHT work if an entire request could be smuggled for instance \tGET, which is a test we have.

As a result the suggested Regex is:
[\r\n]\W*?(?:content-(type|length)|set-cookie|location):

For the various OS commands you have listed. Please see @lifeforms work on completely rewriting that section. He is an official Regex Wizard.

On rule 941310:
The issue is VERY rare, only usable against Tomcat, apparently. In any even the evasion is relevant. The rule was written for a specific bypass. But a more general rule is needed. The goal is not to introduce a very vauge rule that detects on any < or > -- this would be bad. However we don't necessarily want a rule that alerts on any 1/4 or 3/4th.

Orig Rule:
.*¾.*¼.*|.*¼.*¾.*

Suggested new rule:

(?:¾|¼).*(?:¾|¼|>)|(?:¾|¼|<).*(?:¾|¼) 

Example Patterns:

<script>alert(¢XSS¢)¼/script>
<script>alert(¢XSS¢)</script¾
¼script>alert(¢XSS¢)</script>
<script¾alert(¢XSS¢)</script>
~!~<script>alert(¢XSS¢)</script>
¼script¾alert(¢XSS¢)¼/script>
¼script¾alert(¢XSS¢)¼/script¾
<script¾alert(¢XSS¢)¼/script¾

In your text you made some reference to ' creating separate regexs' for this however there are MANY other regexs that address that concern. Here we only care about this very RARE bypass will mitigating the likelihood of FP's

UTF-7 encoding for IE ... wow this one i must admit, i never did see. But there internet is filled with a few examples (thanks microsoft). In any event. I propose making a new rule that is similar to the previous rule:
(?:\+ADw\-|\+AD4\-).*(?:\+ADw\-|\+AD4\-|>)|(?:\+ADw\-|\+AD4\-|<).*(?:\+ADw\-|\+AD4\-)

In general we're not looking to stop all XSS but here just stop basic utf-7 encoded XSS, leaving the remaning filters for the rest. Also note this rule must URLdecode.

Examples:
+ADw-script+AD4-MyMethod()+ADw-/script+AD4-
+ADw-script+AD4-alert(document.location)+ADw-/script+AD4-
%2BADw-script+AD4-alert(document.location)%2BADw-/script%2BAD4-
+ACIAPgA8-script+AD4-alert(document.location)+ADw-/script+AD4APAAi-

More data: http://openmya.hacker.jp/hasegawa/security/utf7cs.html

this should be pushed to 2.x as well.

Regarding ReDOS. ModSecurity includes configurations that allow the user to decide what to do in this situation.
Users can choose what to do in this situation and in fact the default situation is to block the request from being processed by the web server if it trips PCRE limits.
See here: https://github.com/SpiderLabs/ModSecurity/blob/master/modsecurity.conf-recommended#L88

This should have the effect of only preventing access to the DOSer as opposed to the rest of the site. It is unclear how you reached the conclusion that ModSecurity has any issues in this regard.

Rule 942190.
The issue detected is with +’s instead of *’s. Indicating one or more. This is a collection of 11 rules. The issues are present within the 1st alternative

  • (?:\sexec\s+xp_cmdshell)
  • (?:from\W+information_schema\W)
  • (?:exec\s+master.)
  • (?:into[\s+]+(?:dump|out)file\s*?[\"'`])

In this first issue the following will trigger the regex.

  • Exec is short for Execute and therefore this should also be included
  • This must have a space after it according to spec as it is a starting operator.

Improving the check to
We use the \W here because it is possible that plus’s minus’s equal signs might be applicable in addition to spaces and dots before xp_CMDshell

Suggested Rule:
(?:\s*?(?:exec|execute).*?(?:\W)xp_cmdshell)

Example Triggers:

EXEC @result = xp_cmdshell 'dir *.exe'; 
EXEC xp_cmdshell 'copy c:\SQLbcks\AdvWorks.bck
EXEC master..xp_cmdshell 'dir *.exe'' 
EXEC master.dbo.xp_cmdshell 'dir ' + @FileName

In the second issue we investigate the space between from and information_schema. This is in fact required.

Issue three is very similar to issue 1 but only checks for the master. Here it is clear there must be a space after exec however execute is not accounted for. Updating the regex to below:
Suggested Rule:
(?:(exec|execute)\s+master\.)

In issue 4 we have the following

Examples:
http://www.site.com/index.php?id=-725 UNION SELECT 1,passwd,3,4,5 FROM admin INTO DUMPFILE '/path/on/target/file.txt'—

Generally this rule looks fine it is unclear why this additional + is here in the [\s+]+. SQL doesn’t except + and it is URIdecoded so pluses should probably be removed. The plus within the regex has been removed as it wasn’t deemed needed.

Our next rule is 942240. Within this rule we have 3 separate rules

  • (?:alter\s*?\w+.*?character\s+set\s+\w+)
  • ([\"'];\s*?waitfor\s+time\s+[\"'])
  • (?:[\"'];.?:\s?goto)`

Here again we must have at least one space between these elements. However the penultimate rule is of very poor quality and I recommend fixing it:
(?i:(?:alter\s*?\w+.*?(?:character|char)\s+set\s+\w+)|([\"'];?\s?waitfor\s+(?:time|delay)\s+[\"'])|(?:[\"'];.?:\s?goto))`

Section 1 examples:
ALTER TABLE tbl_name CONVERT TO CHARACTER SET charset_name;
ALTER TABLE etape_prospection CONVERT TO CHARACTER SET utf8;
alter x character set d

Section 2 examples
http://www.site.com/vulnerable.php?id=1' waitfor delay '00:00:10'--
";waitfor time '
SELECT first_name,last_name FROM MEMBER WHERE first_name ='joe' AND last_name='frazier' WAITFOR DELAY '00:00:20'

Rule 942350
The rule actually misses a lot of what it needs to catch because of regex failures. The rule restriction
Proposed Update:

(?i:(?:create\s+function\s+.+\s+returns)|(?:;\s*?(?:select|create|rename|truncate|load|alter|delete|update|insert|desc)\s*?[\[(]?\w{2,}))
Examples:

CREATE function pg_sleep(int) RETURNS int AS '/lib/libc.so.6', 'sleep' LANGUAGE 'C' STRICT
create function LockWorkStation returns integer soname 'user32';
create function ExitProcess returns integer soname 'kernel32';

For version 2.x You reported rule 981242 was weak as it could by bypassed with a ". While the part of the regex you highlighted has this feature. The remainder of the regex that you didn't highlight does block this action.

Original Regex:
(?i:(?:[\"'´’‘]\s?(x?or|div|like|between|and)\s?["'´’‘]?\d)|(?:\\\\x(?:23|27|3d))|(?:^.?[\"'´’‘]$)|(?:(?:^["'´’‘\\\\]*?(?:[\d\"'´’‘]+|[^"'´’‘]+[\"'´’‘]))+\s?(?:n?and|x?x?or|div|like|between|and|not|\|\||\&\&)\s?[\w"'´’‘][+&!@(),.-])|(?:[^\w\s]\w+\s*?[|-]\s*?[\"'´’‘]\s?\w)|(?:@\w+\s+(and|x?or|div|like|between|and)\s?["'´’‘\d]+)|(?:@[\w-]+\s(and|x?or|div|like|between|and)\s*?[^\w\s])|(?:[^\w\s:]\s*?\d\W+[^\w\s]\s*?[\"'´’‘].)|(?:\Winformation_schema|table_name\W))`

No recommended changes.

Rule 942260 also 942250
Initial commit which added extra space after not (and was afterwards copypasted into different WAFs) - may I transfer you 8 years back:
https://github.com/PHPIDS/PHPIDS/commit/667e63af93e8fd2ee4df99dd98cb41acdf480906
Watch out - there are 5 affected rules.

in Version 2.x rule 950120 was highlighted. The rule was missing t:lowercase. and t:urldecodeuni.
The new rule should look as follows:

SecRule ARGS "^(?:ht|f)tps?://(.*)$" \     "chain,phase:2,rev:'3',ver:'OWASP_CRS/2.2.9',maturity:'9',accuracy:'9',t:none,t:lowercase,t:urldecodeuni,capture,ctl:auditLogParts=+E,block,msg:'Possible Remote File Inclusion (RFI) Attack: Off-Domain Reference/Link',logdata:'Matched Data: %{TX.0} found within %{MATCHED_VAR_NAME}: %{MATCHED_VAR}',id:'950120',severity:'2',tag:'OWASP_CRS/WEB_ATTACK/RFI'"
        SecRule TX:1 "!@beginsWith %{request_headers.host}" "setvar:'tx.msg=%{rule.msg}',setvar:tx.anomaly_score=+%{tx.critical_anomaly_score},setvar:tx.%{rule.id}-OWASP_CRS/WEB_ATTACK/RFI-%{matched_var_name}=%{tx.1}"

Rule 921120
Full description of found problem here: https://github.com/netty/netty/issues/5535
Proposition looks good

For version 2.x Rule 958056
The issue was a completely arbitrary limit on this regex (how crazy)

Original Regex:

\biframe\b.{0,100}?\bsrc\b

New Regex:
\biframe\b.*?\bsrc\b

Within 2.x, we should push the same fixes to Microsofty issues we pushed to 3.x

CRS version 2: rule 973307

The rule if very weak (also very specific) but it was pointed out that ` can be used to bypass the rule. A minor fix has been put in place

Orig Regex:

(fromcharcode|alert|eval)\s*\(

New Regex:

(?:fromcharcode|alert|eval)\s*(?:\(|`)

in Version 2.x rule 973300 is specifically designed to prevent known HTML tags. This is somewhat the issue with evaluating rules individually, there are other rules that are designed to detect on other XSS issues (100's as you know). As a result, this issue should not be fixed.

Most rules looks good :) I put thumbs up in places where I aggree with suggestions.

  • For version 2.x You reported rule 981242 was weak
    In my example I highlighted the fact of unwanted usage of start and end of the string symbols, and indicated the rule number where I found the snippet. It might be better to avoid such symbols at all. The problem is that bypasses could be found depending on injection.
    Example injection: SELECT 1, '[INJECT]'
    Blocking example: ''a' between '-' and '+
    Include a whitespace before injection and rule will not catch it

Regarding ReDOS
That's true that ReDOS is not a problem for modsecurity, at least I didnt find a way to bypass it.
I only showed that ReDOS problem may exists in real life (other WAFs? backends? frontends?), as some engineers write regexps inaccurately.

Other SQL issues:
Looks good by now as a quick fix, but I do believe that SQL rules have to be updated after several strong runs of good fuzzing.
May I show you some results in this area https://github.com/attackercan/cpp-sql-fuzzer (also to be presented during BHUSA talk)

If I look his paper, I see a certain chance that @attackercan might be promoted to become a regex wizard by the /^IA{2}REW$/ as well.

Add new rule 920270 to CRS 2.x for backwards compatibility

SecRule ARGS|ARGS_NAMES|REQUEST_HEADERS|!REQUEST_HEADERS:Referer "@validateByteRange 1-255"

This issue has timed out as it has not received any update in over 2 years.
If this is still a problem please open a new issue.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

dune73 picture dune73  Â·  6Comments

jeremyjpj0916 picture jeremyjpj0916  Â·  4Comments

dune73 picture dune73  Â·  6Comments

meigea picture meigea  Â·  4Comments

franbuehler picture franbuehler  Â·  5Comments