Hacker News new | ask | show | jobs
by meritt 4263 days ago
These aren't prepared statements. This wouldn't be an issue if they were actual RDBMS prepared statements. These are the bullshit fake prepared statements that PDO emulates by default to achieve cross-database compatibility to offer things like named-parameters (oracle, postgresql support) for databases that only offer positional parameters (mysql, mssql).

It's quite simply shoddy string substitution that's not doing proper escaping, as you pointed out.

2 comments

PDO's prepared statement "emulation" is ridiculous, and turning it off might have blocked some forms of this vulnerability since it would prevent breaking up one query into many. But fixing PDO wouldn't have entirely prevented this fiasco, either.

No amount of prepared statement kung-fu will save you when the querystring itself contains untrustworthy data. Which is exactly what Drupal is doing here. It puts untrusted, potentially non-integer array keys directly into a querystring. Even if Drupal used a database library that supported proper prepared statements, it would have been owned just as well, only slightly less severely.

It's similar to another, much more common misuse of prepared statements: using untrusted values in the column name.

Still, it's always a good idea to set PDO::ATTR_EMULATE_PREPARES to FALSE as soon as you create any instance of that bloody class.

> databases that only offer positional parameters (mysql, mssql).

Something between PHP and MSSQL must not support named parameters, because MSSQL supports them just fine.

So does MySQL, actually. They are just allegedly slow (though I've never benchmarked and I wouldn't be surprised if that's no longer the case).
Actually, Microsoft's SQLSRV PDO driver supports named parameters.