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

Bypass the latest crs v3.1.0-rc3 rules for SQL injection #1167

Closed
seedis opened this issue Aug 10, 2018 · 15 comments
Closed

Bypass the latest crs v3.1.0-rc3 rules for SQL injection #1167

seedis opened this issue Aug 10, 2018 · 15 comments

Comments

@seedis
Copy link

seedis commented Aug 10, 2018

Vulnerability demo(php+mysql+apache)

test.php

<?php 
$id=$_GET['id'];
$conn=new mysqli('localhost','root','root','test');
if(!$conn){
	die("Error to connect database!");
}
$sql="select username from user where id=".$id;
echo "$sql"."</br></br>";
$res=$conn->query($sql);
#while($row=$res->fetch_row())  
if($res){
    $row=$res->fetch_row();
    echo "\t Hello <b>$row[0] </b>,Welcome to login,having a good day!";
    $res->free();
    $conn->close();
}else{
echo "<b>".mysqli_error($conn)."</b>";
}
?>

Download and install the latest v3.1.0-rc3 rules and enable blocking protection for testing.
I found a way to bypass the rules for SQL injection through black box testing.
This method is: {`a`b} , where a is a special function name, such as if, version, etc., and b is the sql statement to be executed.
Using the method to successfully bypass the rules for SQL injection, you can see that the database name was successfully read using the error.

http://127.0.0.1/test.php?id=1  and{`if`updatexml(1,concat(0x3a,(select /*!50000(/*!50000schema_name) from/*!50000information_schema*/.schemata limit 0,1)),1)}

image

In this way, you can use this method to bypass the crs rules and get any content in the database in the vulnerable system.

Please fix this security issue as soon as possible,Thank You.

@seedis seedis changed the title Bypass latest crs for SQL injection Bypass latest crs v3.1.0-rc3 rules for SQL injection Aug 10, 2018
@seedis seedis changed the title Bypass latest crs v3.1.0-rc3 rules for SQL injection Bypass the latest crs v3.1.0-rc3 rules for SQL injection Aug 10, 2018
@theMiddleBlue
Copy link
Contributor

theMiddleBlue commented Aug 10, 2018

Hi @seedis

thanks for sharing this! I've done some tests and I confirm that it works on PL1. By increasing the Paranoia Level (>=2) I can't exploit the SQLi anymore because of 942150 and 942100 rules.

I've changed your PHP script in order to make it works with the WordPress DB as follows:

<?php 

$id=$_GET['id'];
$conn=new mysqli('mysql','root','password','wordpress');

if(!$conn){
        die("Error to connect database!");
}

$sql="select user_login from wp_users where id=".$id;
echo "$sql"."</br></br>";
$res=$conn->query($sql);

if($res){
    $row=$res->fetch_row();

    echo '<pre>';
    print_r($row);
    echo '</pre>';
    
    echo "\t Hello <b>$row[0] </b>,Welcome to login,having a good day!";
    $res->free();
    $conn->close();
} else {
    echo "<b>".mysqli_error($conn)."</b>";
}

And used this payload (blind sqli, and something like user enumeration):

GET /test.php?id=9999%20or+{`if`(2=(select+2+from+wp_users+where+user_login='admin'))}

image

By using PL2 the same payload is blocked by rule 942150 SQL Injection Attack Matched Data: if found within MATCHED_VARS:ARGS:id: 9999 or {if(2=(select 2 from wp_users where user_login='admin'))}. Trying to bypass 942150 by changing if with something else, it is blocked by libinjection.

Probably, it could be intercepted by something like:

SecRule ARGS "@rx {\x60([^\x60]+)\x60" "id:999,\
  phase:1,\
  capture,\
  t:urlDecode,\
  t:removeWhitespace,\
  t:removeComments,\
  block,msg:'SQLi Bypass Attempt #1167',\
  logdata:'Detected using function %{tx.0}'"

With the same payload: 999 SQLi Bypass Attempt #1167 (Detected using function {`if`)

but I don't know if this could lead to many false positives.

@dune73
Copy link
Contributor

dune73 commented Aug 10, 2018

Thank you for reporting @seedis - and for trying this out @theMiddleBlue.

I do not have a PHP server at hand on the quick, so I tried this out by hand with curl. I took

http://127.0.0.1/test.php?id=1  and{`if`updatexml(1,concat(0x3a,(select /*!50000(/*!50000schema_name) from/*!50000information_schema*/.schemata limit 0,1)),1)}

But curl is really upset.

Then I took

curl 'localhost/index.html?id=9999%20or+{`if`(2(select+2+from+wp_users+where+user_login="admin"))}'

and I got the following alerts:

942100 PL1 SQL Injection Attack Detected via libinjection
942150 PL2 SQL Injection Attack
942200 PL2 Detects MySQL comment-/space-obfuscated injections and backtick termination
942260 PL2 Detects basic SQL authentication bypass attempts 2/3
942410 PL2 SQL Injection Attack
942480 PL2 SQL Injection Attack
942490 PL3 Detects classic SQL injection probings 3/3
942431 PL3 Restricted SQL Character Anomaly Detection (args): # of special characters exceeded (6)
920273 PL4 Invalid character in request (outside of very strict set)
942432 PL4 Restricted SQL Character Anomaly Detection (args): # of special characters exceeded (2)

So 942100 detects this just fine in my case.

Can you guys give me a naked curl call that bypasses CRS and still executes?

Other than that, detecting on PL2 is not that bad. We will always have bypasses. Having a bypass at PL1 but being detected at PL2 is inline with out project goals.

But it goes without saying that changing a rule to detect this (as well) could make sense if we can avoid false positives.

@theMiddleBlue
Copy link
Contributor

same behavior here :/ using curl it is detected by 942100 but it if I send it using BurpSuite it works. don't know why... I'm trying to figure it out.

@dune73
Copy link
Contributor

dune73 commented Aug 10, 2018

Glad you checked.

Could you run a tcpdump when doing burp? In cases where curl interferes with the payload, we can also use netcat (nc) for raw socket writing.

@seedis
Copy link
Author

seedis commented Aug 10, 2018

I used the default level when testing.
In fact, {`a`b} is just used to bypass the annotations in mysql. The default level annotations are not available, but in {`a`b}, you can use annotations to make rules. Bypassed. You can see that the example I gave gives /*!50000*/ such an annotation. If you don't use the annotation, you can't bypass the rules. Just like the test case you gave before, you need to combine the annotations. To bypass the rules.@dune73 @theMiddleBlue

@theMiddleBlue
Copy link
Contributor

ok, interesting findings:

  1. the right payload to use with curl is curl -v "http://wordpress/test.php?id=9999%20or+\{%20%20%20%60if%60(2=(select+2+from+wp_users+where+user_login=\'admin\'))\}" (I've encoded ` with %60 and escaped { and })

  2. If I request it without compression (without Accept-Encoding: gzip, deflate) I can bypass libinjection but I went blocked by 951230 mysql SQL Information Leakage (Matched Data: You have an error in your SQL syntax; found within RESPONSE_BODY: select user_login from wp_users where id=9999 or { if(2=(select 2 from wp_users where user_login=\'admin\'))}</br></br><b>You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '\'admin\'))}' at line 1</b>) and (IMHO) this is f*cking awesome 😄

  3. Using this: curl -s -H $'Accept-Encoding: gzip, deflate' $'http://wordpress/test.php?id=9999%20or+\{%20%20%20%60if%60(2=(select+2+from+wp_users+where+user_login=\'admin\'))\}' | gunzip -- it works as expected

@lifeforms
Copy link
Contributor

lifeforms commented Aug 10, 2018

@dune73 Urlencoded, the payload becomes:

http://localhost/test.php?id=1%20%20and%7B%60if%60updatexml%281%2Cconcat%280x3a%2C%28select%20%2F%2A%2150000%28%2F%2A%2150000schema_name%29%20from%2F%2A%2150000information_schema%2A%2F.schemata%20limit%200%2C1%29%29%2C1%29%7D

I noticed that this goes undetected at PL1.

It seems to be a false negative in libinjection:

./check '1  and{`if`updatexml(1,concat(0x3a,(select /*!50000(/*!50000schema_name) from/*!50000information_schema*/.schemata limit 0,1)),1)}'
not sqli

But it also seems a false negative in our own rule 942140, which should detect the mention of information_schema blacklisted database name, I wonder why this doesn't trigger...

@seedis
Copy link
Author

seedis commented Aug 10, 2018

@dune73 @theMiddleBlue
If you need to use Boolean injection, you need to wrap the keywords such as select, from, etc. with the extension annotation /*!50000*/ in mysql.
image

@dune73
Copy link
Contributor

dune73 commented Aug 10, 2018

I confirm. We have a PL1 bypass. Thank you @theMiddleBlue and @lifeforms.

@lifeforms: Can you open an issue with libinjection?

@themiddle: The whole encoding trickery is indeed funny, but you should disable deflation towards the application on the ModSec level. With that 951230 becomes always active, but this still leaves us with the blind sqli.

What do we do? Leave it as is (remember: we detect at PL2) and wait for libinjection to catch up. OR extend an existing rule (which one?) or create a new one?

@csanders-git
Copy link
Contributor

Defense in depth seems to say we extend our rule and hope lib injection fixes it

@dune73
Copy link
Contributor

dune73 commented Aug 10, 2018

@seedis: Thank you for explaining this extension annotation. This is dangerous and tricky to detect for us. Good to have sql experts at hand that can explain things as this to us.

@lifeforms
Copy link
Contributor

I'll report it to libinjection. But libinjection > ModSec 2.9.4 is a long cycle, we can't depend on that.

Maybe @franbuehler could check why information_schema is not detected by us. It seems that we have this string in multiple data files:

util/regexp-assemble/regexp-942140.data
3:information_schema\b

util/regexp-assemble/regexp-942330.data
45:\Winformation_schema

Why is it slipping past us?

@spartantri
Copy link
Contributor

@lifeforms 942140 doesn't match because there is no boundary before information_schema the 5000 is part of the same word so we may add an additional optional word prefix to 942140 and it will work.

something like this:
(?i:\b(?:\w+)?(?:(?:m(?:s(?:ys(?:ac(?:cess(?:objects|storage|xml)|es)|(?:relationship|object|querie)s|modules2?)|db)|aster\.\.sysdatabases|ysql\.db)|pg_(?:catalog|toast)|information_schema|northwind|tempdb)\b|s(?:(?:ys(?:\.database_name|aux)|qlite(?:_temp)?_master)\b|chema(?:_name\b|\W*\())|d(?:atabas|b_nam)e\W*\())

942440 at PL2 is there to detect the /*!

@franbuehler
Copy link
Contributor

franbuehler commented Aug 10, 2018

As @lifeforms mentioned, the string information_schema can be found in different source files:

942140:information_schema\b
942190:from\W+information_schema\W
942330:\Winformation_schema

The problem in rule 942140 is, as @spartantri said, the word boundary: His solution with the (?:\w+)? works!
The problem with the rules 942190 and 942330 is the required non-word character in front of information_schema.
Rule 942190 could also be extended with the (?:\w+)?: from\W+(?:\w+)?information_schema\W.
Rule 942330 too: \W(?:\w+)?information_schema.

But that only solves the problem related to information_schema.

Either we extend all our sqli rules with this regexp or we make a new rule to detect extension annotation. I am not an sql pro (had a look at: https://dev.mysql.com/doc/refman/8.0/en/comments.html), but maybe something like that would work to detect (some) extensions:

\/\*[!+](?:[\w\s=_\-()]+)?\*\/

Tested with:
1 and{ifupdatexml(1,concat(0x3a,(select /*!50000(/*!50000schema_name) from/*!50000information_schema*/.schemata limit 0,1)),1)}
and from: https://dev.mysql.com/doc/refman/8.0/en/comments.html:
SELECT /*! STRAIGHT_JOIN */ col1 FROM table1,table2 WHERE ...
CREATE TABLE t1(a INT, KEY (a)) /*!50110 KEY_BLOCK_SIZE=1024 */;
SELECT /*+ BKA(t1) */ FROM ... ;

I am not sure about false positives.
But the minimal string needed to trigger the regexp is: /*!*/ or /*+*/.
In my opinion this is not a very commonly used, legitimate string.

The rule 942440 from PL2 catches both /*! and */ independently. This is more prone to false positives.

I would rather write a new rule (maybe even on PL1) to catch these extensions, than to extend all sql rules.

@dune73
Copy link
Contributor

dune73 commented Aug 11, 2018

I agree on the new rule. Yesterday, I mentioned crazy special char combinations like in #425. But thinking some more about this, I no longer propose to combine this. Extension annotation is clearly a specialty of the mysql family. So the rule should stay with the sql rules.

We think there should not be too many FPs, but adding it to PL1 is still post-feature-freeze for 3.1 and we really do not know. So I do not think we should do that. Maybe for PL2 and lower to PL1 in the future.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants