Skip Menu |
 

This queue is for tickets about the DBD-mysql CPAN distribution.

Report information
The Basics
Id: 126014
Status: open
Priority: 0/
Queue: DBD-mysql

People
Owner: Nobody in particular
Requestors: MLEHMANN [...] cpan.org
Cc: marco [...] nethype.de
pali [...] cpan.org
AdminCc:

Bug Information
Severity: Normal
Broken in: 4.046
Fixed in: (no value)



CC: marco [...] nethype.de
Subject: security issue: placeholders will string-interpolate without quoting, executing parameters as sql
Download (untitled) / with headers
text/plain 1.4k
Hi! We currently run into a DBD::mysql where placeholders (?) in prepared statements get replaced with the string value of the parameter without any quoting, leading to parameter values being executed as sql. I don't have a testcase, but looking at the code in dbdimp.C parse_params, it's quite obvious that this is the case under certain conditions. If you follow the logic for case '?', then the parameter value will be interpolated without any quoting when bind_type_guessing is false and the column is numeric - SQL_NUMERIC will cause is_num to be set, nothing will re-set it, and the code will then take the string(!) value of the parameter value and interpolate it as sql commands instead of quoting it. For example, the following code will probably trigger this issue: $st = $dbh->prepare ("delete from users where uid = ?"); $st->bind_param (1, undef, { TYPE => DBI::SQL_NUMERIC }); $st->execute ("uid"); Which would result in: delete from users where uid = uid I haven't tested the code above, but following the logic for case '?' is quite easy and somehow it must be possible to have bind_type_guessing off, while having a numeric type for a parameter. The code is overall quite weirdly inefficient (why does it call parse_number when is_num is false for example), so this is probably the result of some refactoring, having been undetected because bind_param is rarely used. I think this is a security bug because ? placeholders should NEVER EVER treat the bound value as sql code and execute it.
Download (untitled) / with headers
text/plain 326b
Just FYI, moving the is_num = 0 from after the "if (bind_type_guessing)" to before seems to fix this, although the performance could probably be further improved by calling parse_number only when is_num is true. Beware, though, I haven't looked at what parse_number actually does - there could be further security bugs inside.
Download (untitled) / with headers
text/plain 1.7k
On Uto Aug 07 08:42:15 2018, MLEHMANN wrote: Show quoted text
> Hi! > > We currently run into a DBD::mysql where placeholders (?) in prepared > statements get replaced with the string value of the parameter without > any quoting, leading to parameter values being executed as sql. > > I don't have a testcase, but looking at the code in dbdimp.C > parse_params, it's quite obvious that this is the case under certain > conditions. > > If you follow the logic for case '?', then the parameter value will be > interpolated without any quoting when bind_type_guessing is false and > the column is numeric - SQL_NUMERIC will cause is_num to be set, > nothing will re-set it, and the code will then take the string(!) > value of the parameter value and interpolate it as sql commands > instead of quoting it. > > For example, the following code will probably trigger this issue: > > $st = $dbh->prepare ("delete from users where uid = ?"); > $st->bind_param (1, undef, { TYPE => DBI::SQL_NUMERIC }); > $st->execute ("uid"); > > Which would result in: > > delete from users where uid = uid > > I haven't tested the code above, but following the logic for case '?' > is quite easy and somehow it must be possible to have > bind_type_guessing off, while having a numeric type for a parameter. > > The code is overall quite weirdly inefficient (why does it call > parse_number when is_num is false for example), so this is probably > the result of some refactoring, having been undetected because > bind_param is rarely used. > > I think this is a security bug because ? placeholders should NEVER > EVER treat the bound value as sql code and execute it.
Hi! This problem should be already fixed in DBD::MariaDB driver: https://github.com/gooddata/DBD-MariaDB There were fixed also other issues with parsing parameters and placeholders.


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.