Closed (fixed)
Project:
Protected Node
Version:
6.x-1.5
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Reporter:
Created:
12 Jul 2010 at 20:46 UTC
Updated:
1 May 2011 at 03:31 UTC
The selection statement in protected_node_requirements() is vulnerable to SQL injection. I'm not filing this with the security team because all permissions are defined be modules, however it's possible that a permission might accidentally cause a problem.
$likes = array();
foreach ( $perms as $perm ) {
$likes[] = "perm LIKE '%" . $perm . "%'";
}
$sql = "SELECT rid FROM {permission} WHERE " . implode( ' OR ', $likes );
$roles = db_affected_rows( db_query( $sql ) );
Since perm is not escaped in any way, it's possible that a permission may inaccurately be matched or end the SQL statement accidentally (such as if the permission contains a single quote), causing a SQL error.
Instead this should be:
$likes = array();
foreach ($perms as $perm) {
$likes[] = "perm LIKE '%%s%'";
}
$sql = "SELECT rid FROM {permission} WHERE " . implode(' OR ', $likes);
$roles = db_affected_rows(db_query($sql, $perms));
There's also a function specifically for this kind of replacement, but I couldn't figure out how it would apply in this situation: http://api.drupal.org/db_placeholders.
Comments
Comment #1
AlexisWilke commentedI used your code because I like it. 8-)
But that was not an SQL injection because:
The $perm contents are taken from protected_node_perm() which is limited to:
(a) the module permissions (and we know we don't inject anything bad here); and
(b) node type names which are limited to [a-z0-9_]+
So all names were under control and could not generate an SQL injection. This being said, fixing such things is always a good idea (in case someone was thinking to do a copy + paste...)
Thank you.
Alexis Wilke
See:
http://drupalcode.org/project/protected_node.git/commit/e2efd11