6 ms·
Why does Drupal use string manipulation to generate SQL statements instead of bind parameters? It is 2014, hasn't this been best practice since at least the lat
by UnoriginalGuy 12y ago
Why does Drupal use string manipulation to generate SQL statements instead of bind parameters? It is 2014, hasn't this been best practice since at least the late 1990s? Wouldn't all security audits, including automated ones, pick this up?
And before someone makes a "joke" about PHP, they're using the PDO framework which supports bindParam(). Seems like woful incompetence in the Drupal codebase to me.
Exploit in detail:
https://www.sektioneins.de/en/advisories/advisory-012014-drupal-pre-auth-sql-injection-vulnerability.html https://www.sektioneins.de/en/advisories/advisory-012014-dru...
- fuligo 12y agoGenerating SQL statements always involves string manipulation at some stage, doesn't it? I mean at some point even PDO has to send query strings to the SQL server. The question is whether Drupal really needed to implement their own engine for this, though.
- UnoriginalGuy 12y ago> Generating SQL statements always involves string manipulation at some stage, doesn't it? I mean at some point even PDO has to send query strings to the SQL server. If your database supports it set PDO::ATTR_EMULATE_PREPARES to false. If not, I'd still take PDO's implementation over a million different implementations which other projects come up with such as this one.
- ams6110 12y agoLegacy? Tons of code to change, and users more interested in new features? No tests? Developer conceit about their own abilities to "do it right"? All the usual reasons any project has old code that exhibits bad practices by current standards.
- krapp 12y ago>Why does Drupal use string manipulation to generate SQL statements instead of bind parameters? Is 'elegance creep' a thing? I would be willing to bet money I don't have, that it's because someone thought it was more elegant and clever, and therefore just better.
- jiggy2011 12y agoThe query in question is using an IN clause which takes a tuple of values. I don't think bindParam will allow you to bind an array into a tuple , the only way to pass the tuple is by forming it as a string and adding that to the query string.
- opendais 12y agoYou can write a function for it using ?'s being generated inside your code only. Its perfectly safe to do it that want and allows you to bind parameters. The only explanation for not doing that is ignorance and/or incompetence. Mistakes happen but to claim its a language problem is incorrect. http://php.net/manual/en/pdostatement.bindparam.php http://php.net/manual/en/pdostatement.bindparam.php <?php /* Execute a prepared statement by binding PHP variables */ $calories = 150; $colour = 'red'; $sth = $dbh->prepare('SELECT name, colour, calories FROM fruit WHERE calories < ? AND colour = ?'); $sth->bindParam(1, $calories, PDO::PARAM_INT); $sth->bindParam(2, $colour, PDO::PARAM_STR, 12); $sth->execute(); ?> So what you do is you write a function to convert the array to a series of ?'s for the IN clause/tuple and then iterate through the array to bind the parameters.
- jiggy2011 12y agoI don't think that would work in this case, because there is no way to bind a PHP array to an SQL tuple. Available param types includes string , int , bool but no array or tuple type. http://php.net/manual/en/pdo.constants.php http://php.net/manual/en/pdo.constants.php You can write "SELECT calories FROM fruit WHERE name IN (? , ?, ?)" and then bind the parameters as strings but this will only work in cases where the length of the tuple is known and fixed. If you need to allow for a variable length tuple then you will need to concatenate the query string yourself.
- SapphireSun 12y agoJust use a placeholder like (??) and flatten everything out. Translate (??) and array(1,2,3) into (?,?,?) with 1, 2, 3 as the binds.
- lucio 12y agoThey're using bind-params, but instead of using "?" as placeholder, they were using "named" placeholders, and, the placeholder name was constructed using the associative array key (externally received). The mistake was using a external provided value instead of using "?" or generating independent names for placeholders.