msphpsql icon indicating copy to clipboard operation
msphpsql copied to clipboard

This PDO driver lacks security measures to help prevent SQL Injection

Open david-garcia-garcia opened this issue 9 years ago • 4 comments

The MySQL PDO driver has the PDO::MYSQL_ATTR_MULTI_STATEMENTS initialization parameter that when set to TRUE will limit the PDO driver to executing single statements.

See the PHP issue here:

https://bugs.php.net/bug.php?id=68424

And the pull request:

https://github.com/php/php-src/pull/896

This availability of such a setting would have - among other things - mitigated the impact of the Drupagedon SQL Injection vulnerability that was disclosed last year. The patch was indeed pushed forward by Drupal maintainers.

david-garcia-garcia avatar Oct 09 '15 01:10 david-garcia-garcia

@Beakerboy just wondering if you know about this feature request? The PDO_SQLSRV driver does not emulate prepare by default like MySQL PDO driver.

yitam avatar Aug 05 '20 00:08 yitam

Wow, I’m thrilled that you summoned me to an issue, as if I may be a person who’s opinion is worth hearing. To be honest, I am a chemical engineer, just trying to keep his data website working.

Drupal has other measures in place to counteract this. The DBAL will throw exceptions if a query has a semicolon in it...which is actually a problem because Microsoft REQUIRES a terminating semicolon for MERGE statements.

If this is a feature that other drivers have, and the justification is sound, you should probably do it too.

Beakerboy avatar Aug 05 '20 00:08 Beakerboy

Thank you so much @Beakerboy! Your prompt reply is very much appreciated.

yitam avatar Aug 05 '20 00:08 yitam

Hi @david-garcia-garcia

There are many possible ways to do SQL injection and using multiple statements is only a subset.

The best way to avoid that is to use prepared statements and/or parameterized queries, as explained here.

Besides, when using pdo_sqlsrv or pdo_odbc, the users can write batch queries without using semicolons, as shown in the following script using pdo_odbc and AdventureWorks sample DB:

$pdo = new PDO("odbc:driver={$driver};server=$server;database=$database", $uid, $pwd);

$stmt = $pdo->prepare("SELECT TOP 1 Name FROM [Production].[Product] SELECT TOP 1 Name FROM [Sales].[Store]");
$stmt->execute();
$array = $stmt->fetchAll(PDO::FETCH_ASSOC);
var_dump($array);

$stmt->nextRowset();
$array = $stmt->fetchAll(PDO::FETCH_ASSOC);
var_dump($array);

The results are

array(1) {
  [0]=>
  array(1) {
    ["Name"]=>
    string(15) "Adjustable Race"
  }
}
array(1) {
  [0]=>
  array(1) {
    ["Name"]=>
    string(20) "Next-Door Bike Store"
  }
}

There is no guaranteed way to correctly detect a batch query, so even if we add a flag like PDO::MYSQL_ATTR_MULTI_STATEMENTS it may not be very useful. This feature request is thus categorized as "low priority".

yitam avatar Aug 19 '20 01:08 yitam