Problem: With PDO the mysql driver emulated allows multiple queries,
where non-emulated does not. This makes SQL injections with PDO mysql
potentially much more damaging.
Suggested solution: add a PDO attribute that could be set on a
connection or a driver option for PDO::prepare to enforce the limit of
a single query being prepared or run.
Happy to open a bug report/feature request, but looking for feedback first.
More background:
The Drupal project recently had a serious SQL injection vulnerability
uncovered: https://www.drupal.org/SA-CORE-2014-005
A technical discussion of it:
http://blog.ircmaxell.com/2014/10/a-lesson-in-security.html
To be brief, from Drupal 6.x to Drupal 7.x, we converted from wrappers
on mysqli_query and pg_query to a database layer that extends from
PDO.
The most common setup is using PDO mysql and emulated prepared
statements. This turns out to have made the potential damage to
Drupal 7 much greater when an SQL injection is discovered since the
emulated prepare allows multiple SQL statements, hence a single SELECT
can be modified to a SELECT followed by one or more INSERT or UPDATE
statements. mysqli only supports a single statement.
Since this situation probably affects many web applications using PDO,
I'd like to see if a new PDO attribute could be added which would e.g.
cause the driver to throw an exception if multiple statements were
sent at once. Ideally, this could be backported to current releases.
-Peter Wolanin
Suggested solution: add a PDO attribute that could be set on a
connection or a driver option for PDO::prepare to enforce the limit of
a single query being prepared or run.
The issue is that disabling multi-query implicitly also disables support
for stored procedures as the same flag configures handling of operations
with multiple result sets. So this probably needs more thoughts
especially in order to get "similar" behavior with different
databases ... can you add a feature request in the bug tracker for this?
johannes
I've added a pull request here with a proposal to add the attribute at
connection time: https://github.com/php/php-src/pull/896
I think given PHP users the option to do this is really critical for
securing against SQL injection, and giving more consistent behavior between
native and emulated prepares.
From my reading of the mysql API, enabling multi-query implicitly enables
multi results, but it's also possible to enable multi results separately,
and I've left it as is, explicitly enabled, in the patch.
Do you have an example of a stored procedure to test?
Thanks,
Peter
On Mon, Nov 10, 2014 at 12:02 PM, Johannes Schlüter johannes@schlueters.de
wrote:
Suggested solution: add a PDO attribute that could be set on a
connection or a driver option for PDO::prepare to enforce the limit of
a single query being prepared or run.The issue is that disabling multi-query implicitly also disables support
for stored procedures as the same flag configures handling of operations
with multiple result sets. So this probably needs more thoughts
especially in order to get "similar" behavior with different
databases ... can you add a feature request in the bug tracker for this?johannes
--
Peter M. Wolanin, Ph.D. : Momentum Specialist, Acquia. Inc.
peter.wolanin@acquia.com : 781-313-8322
Added as a feature request also: https://bugs.php.net/bug.php?id=68424
I'm a little unclear about the preferred workflow for using pull requests
vs. bugs.php.net - it seems liek most everything released ends up referring
to an issue on bugs.php.net?
-Peter
On Thu, Nov 13, 2014 at 8:21 PM, Peter Wolanin peter.wolanin@acquia.com
wrote:
I've added a pull request here with a proposal to add the attribute at
connection time: https://github.com/php/php-src/pull/896I think given PHP users the option to do this is really critical for
securing against SQL injection, and giving more consistent behavior between
native and emulated prepares.From my reading of the mysql API, enabling multi-query implicitly enables
multi results, but it's also possible to enable multi results separately,
and I've left it as is, explicitly enabled, in the patch.Do you have an example of a stored procedure to test?
Thanks,
Peter
On Mon, Nov 10, 2014 at 12:02 PM, Johannes Schlüter <
johannes@schlueters.de> wrote:Suggested solution: add a PDO attribute that could be set on a
connection or a driver option for PDO::prepare to enforce the limit of
a single query being prepared or run.The issue is that disabling multi-query implicitly also disables support
for stored procedures as the same flag configures handling of operations
with multiple result sets. So this probably needs more thoughts
especially in order to get "similar" behavior with different
databases ... can you add a feature request in the bug tracker for this?johannes
--
Peter M. Wolanin, Ph.D. : Momentum Specialist, Acquia. Inc.
peter.wolanin@acquia.com : 781-313-8322
--
Peter M. Wolanin, Ph.D. : Momentum Specialist, Acquia. Inc.
peter.wolanin@acquia.com : 781-313-8322
yeah, the consensus was to create feature request on bugs.php.net for PRs
(and link the PR from the bugtracker), so that we each NEWS entry can link
to a bug#.
On Fri, Nov 14, 2014 at 4:04 PM, Peter Wolanin peter.wolanin@acquia.com
wrote:
Added as a feature request also: https://bugs.php.net/bug.php?id=68424
I'm a little unclear about the preferred workflow for using pull requests
vs. bugs.php.net - it seems liek most everything released ends up
referring
to an issue on bugs.php.net?-Peter
On Thu, Nov 13, 2014 at 8:21 PM, Peter Wolanin peter.wolanin@acquia.com
wrote:I've added a pull request here with a proposal to add the attribute at
connection time: https://github.com/php/php-src/pull/896I think given PHP users the option to do this is really critical for
securing against SQL injection, and giving more consistent behavior
between
native and emulated prepares.From my reading of the mysql API, enabling multi-query implicitly enables
multi results, but it's also possible to enable multi results separately,
and I've left it as is, explicitly enabled, in the patch.Do you have an example of a stored procedure to test?
Thanks,
Peter
On Mon, Nov 10, 2014 at 12:02 PM, Johannes Schlüter <
johannes@schlueters.de> wrote:Suggested solution: add a PDO attribute that could be set on a
connection or a driver option for PDO::prepare to enforce the limit of
a single query being prepared or run.The issue is that disabling multi-query implicitly also disables support
for stored procedures as the same flag configures handling of operations
with multiple result sets. So this probably needs more thoughts
especially in order to get "similar" behavior with different
databases ... can you add a feature request in the bug tracker for this?johannes
--
Peter M. Wolanin, Ph.D. : Momentum Specialist, Acquia. Inc.
peter.wolanin@acquia.com : 781-313-8322--
Peter M. Wolanin, Ph.D. : Momentum Specialist, Acquia. Inc.
peter.wolanin@acquia.com : 781-313-8322
--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
Any other input on this pull request? Are there tests that should be
duplicated to run in single vs multi query mode?
-Peter
yeah, the consensus was to create feature request on bugs.php.net for PRs
(and link the PR from the bugtracker), so that we each NEWS entry can link
to a bug#.On Fri, Nov 14, 2014 at 4:04 PM, Peter Wolanin peter.wolanin@acquia.com
wrote:Added as a feature request also: https://bugs.php.net/bug.php?id=68424
I'm a little unclear about the preferred workflow for using pull requests
vs. bugs.php.net - it seems liek most everything released ends up
referring
to an issue on bugs.php.net?-Peter
On Thu, Nov 13, 2014 at 8:21 PM, Peter Wolanin peter.wolanin@acquia.com
wrote:I've added a pull request here with a proposal to add the attribute at
connection time: https://github.com/php/php-src/pull/896I think given PHP users the option to do this is really critical for
securing against SQL injection, and giving more consistent behavior
between
native and emulated prepares.From my reading of the mysql API, enabling multi-query implicitly
enables
multi results, but it's also possible to enable multi results
separately,
and I've left it as is, explicitly enabled, in the patch.Do you have an example of a stored procedure to test?
Thanks,
Peter
On Mon, Nov 10, 2014 at 12:02 PM, Johannes Schlüter <
johannes@schlueters.de> wrote:Suggested solution: add a PDO attribute that could be set on a
connection or a driver option for PDO::prepare to enforce the limit
of
a single query being prepared or run.The issue is that disabling multi-query implicitly also disables
support
for stored procedures as the same flag configures handling of
operations
with multiple result sets. So this probably needs more thoughts
especially in order to get "similar" behavior with different
databases ... can you add a feature request in the bug tracker for
this?johannes
--
Peter M. Wolanin, Ph.D. : Momentum Specialist, Acquia. Inc.
peter.wolanin@acquia.com : 781-313-8322--
Peter M. Wolanin, Ph.D. : Momentum Specialist, Acquia. Inc.
peter.wolanin@acquia.com : 781-313-8322--
Ferenc Kovács
@Tyr43l - http://tyrael.hu
--
Peter M. Wolanin, Ph.D. : Momentum Specialist, Acquia. Inc.
peter.wolanin@acquia.com : 781-313-8322
On Mon, Nov 10, 2014 at 12:02 PM, Johannes Schlüter <
johannes@schlueters.de> wrote:Suggested solution: add a PDO attribute that could be set on a
connection or a driver option for PDO::prepare to enforce the limit
of
a single query being prepared or run.The issue is that disabling multi-query implicitly also disables
support
for stored procedures as the same flag configures handling of
operations
with multiple result sets. So this probably needs more thoughts
especially in order to get "similar" behavior with different
databases ... can you add a feature request in the bug tracker for
this?johannes
I've just added another test to address the concern from Johannes that
setting this flag would interfere with stored procedures.
It's a partial copy of the existing
ext/pdo_mysql/tests/pdo_mysql_stmt_nextrowset.phpt which has an
expected fail.
The new test uses just native prepare (which does not fail) and runs
the same test for setting the new connection attribute to either false
or true creating a stored procedure that returns multiple result sets.
see: https://github.com/php/php-src/pull/896
Thanks,
Peter