Bug in ssp.class.php
Bug in ssp.class.php
I believe there is a bug in the ssp.class.php add_bindings function. I'm looking at the version here: https://github.com/DataTables/DataTablesSrc/blob/master/examples/server_side/scripts/ssp.class.php
static function add_bindings(&$a, $vals)
{
foreach($vals['bindings'] as $key => $value) {
$bindings[] = array(
'key' => $key,
'val' => $value,
'type' => PDO::PARAM_STR
);
}
}
Note that $a is passed by reference, but $a is not used in the function - a new array named $bindings is created.
All line numbers are the line numbers found on this page: https://github.com/DataTables/DataTablesSrc/blob/master/examples/server_side/scripts/ssp.class.php
Also note that on line 387, the variable $bindings is passes to sql_exec, but on line 375 the variable $whereAllBindings is passed to self::add_bindings, not $bindings, so even if the add_bindings function is working correctly, the returned value(s) aren't added the the $bindings array, and therefore aren't included in the call to self::sql_exec on line 387. I solved this issue by adding the following line immediately after the call to add_bindings on line 375:
$bindings = array_merge( $bindings, $whereAllBindings );
Becasue $whereAllBindings is needed to get the total record count and needs to contain only the whereAll bindings.
The relevant section of code:
// Likewise for whereAll
if ( $whereAll ) {
$str = $whereAll;
if ( is_array($whereAll) ) {
$str = $whereAll['condition'];
if ( isset($whereAll['bindings']) ) {
self::add_bindings($whereAllBindings, $whereAll['bindings']);
}
}
$where = $where ?
$where .' AND '.$str :
'WHERE '.$str;
$whereAllSql = 'WHERE '.$str;
}
// Main query to actually get the data
$data = self::sql_exec( $db, $bindings,
"SELECT `".implode("`, `", self::pluck($columns, 'db'))."`
FROM `$table`
$where
$order
$limit"
);
It kind of makes me wonder if a lot of developers aren't using bound parameters like they should and are thus opening themselves to SQL injection attacks, or I could be completely off base here due to lack of coffee...
Replies
There is certainly something wrong there - thank you for flagging this up. I'll commit changes tomorrow when I'm back in the office.
I would actually strongly advocate using the Editor PHP libraries for server-side processing, rather than ssp.class.php, which I consider to be a demo only. The Editor PHP libraries are open source and can be freely used while providing a lot more functionality than then ssp.class.php.
I'll maybe add some comments to the demo to this effect. That said, there is an error there and I'll look at it!
Allan