Commit 2e2f34ab authored by Francois Marier's avatar Francois Marier
Browse files

Remove the ability for blocktypes to inject straight SQL into a query



This was not exploitable, but will reduce the risk that a SQL injection
will be accidentally introduced in the future.
Signed-off-by: default avatarFrancois Marier <francois@catalyst.net.nz>
parent d216a8c0
......@@ -183,15 +183,6 @@ class PluginBlocktypeBlog extends PluginBlocktype {
// return $artefact;
//}
/**
* Optional method. If specified, changes the order in which the artefacts are sorted in the artefact chooser.
*
* This is a valid SQL string for the ORDER BY clause. Fields you can sort on are as per the artefact table
*/
//public static function artefactchooser_get_sort_order() {
// return 'parent, ctime DESC';
//}
public static function default_copy_type() {
return 'nocopy';
}
......
......@@ -140,8 +140,6 @@ class PluginBlocktypeBlogpost extends PluginBlocktype {
}
public static function artefactchooser_element($default=null) {
$extrajoin = ' JOIN {artefact_blog_blogpost} ON {artefact_blog_blogpost}.blogpost = a.id ';
$element = array(
'name' => 'artefactid',
'type' => 'artefactchooser',
......@@ -151,8 +149,6 @@ class PluginBlocktypeBlogpost extends PluginBlocktype {
'limit' => 10,
'selectone' => true,
'artefacttypes' => array('blogpost'),
'extrajoin' => $extrajoin,
'extracols' => '1 - {artefact_blog_blogpost}.published AS draft',
'template' => 'artefact:blog:artefactchooser-element.tpl',
);
return $element;
......@@ -180,7 +176,8 @@ class PluginBlocktypeBlogpost extends PluginBlocktype {
* This is a valid SQL string for the ORDER BY clause. Fields you can sort on are as per the artefact table
*/
public static function artefactchooser_get_sort_order() {
return 'parent, ctime DESC';
return array(array('fieldname' => 'parent'),
array('fieldname' => 'ctime', 'order' => 'DESC'));
}
public static function default_copy_type() {
......
......@@ -98,15 +98,6 @@ class PluginBlocktypeRecentposts extends PluginBlocktype {
);
}
/**
* Optional method. If specified, changes the order in which the artefacts are sorted in the artefact chooser.
*
* This is a valid SQL string for the ORDER BY clause. Fields you can sort on are as per the artefact table
*/
public static function artefactchooser_get_sort_order() {
return 'title';
}
public static function default_copy_type() {
return 'nocopy';
}
......
......@@ -3,7 +3,7 @@
<td style="width: 20px;" rowspan="2">
{$formcontrols}
</td>
<th><label for="{$elementname}_{$artefact->id}">{if $artefact->blog}{$artefact->blog|escape}: {/if}{$artefact->title|escape}{if $artefact->draft} [{str tag=draft section=artefact.blog}]{/if}</label></th>
<th><label for="{$elementname}_{$artefact->id}">{if $artefact->blog}{$artefact->blog|escape}: {/if}{$artefact->title|escape}</label></th>
</tr>
<tr>
<td>{if $artefact->description}{$artefact->description}{/if}</td>
......
......@@ -117,7 +117,7 @@ class PluginBlocktypeFolder extends PluginBlocktype {
}
public static function artefactchooser_get_sort_order() {
return 'parent, title';
return array(array('fieldname' => 'parent'), array('fieldname' => 'title'));
}
public static function filebrowser_element(&$instance, $default=array()) {
......
......@@ -93,9 +93,6 @@ class PluginBlocktypeHtml extends PluginBlocktype {
}
public static function artefactchooser_element($default=null) {
$extraselect = 'filetype IN (' . join(',', array_map('db_quote', self::get_allowed_mimetypes())) . ')';
$extrajoin = ' JOIN {artefact_file_files} ON {artefact_file_files}.artefact = a.id ';
return array(
'name' => 'artefactid',
'type' => 'artefactchooser',
......@@ -105,7 +102,6 @@ class PluginBlocktypeHtml extends PluginBlocktype {
'limit' => 10,
'artefacttypes' => array('file'),
'template' => 'artefact:file:artefactchooser-element.tpl',
'extraselect' => $extraselect,
);
}
......
......@@ -110,9 +110,6 @@ class PluginBlocktypeInternalmedia extends PluginBlocktype {
}
public static function artefactchooser_element($default=null) {
$extraselect = 'filetype IN (' . join(',', array_map('db_quote', self::get_allowed_mimetypes())) . ')';
$extrajoin = ' JOIN {artefact_file_files} ON {artefact_file_files}.artefact = a.id ';
return array(
'name' => 'artefactid',
'type' => 'artefactchooser',
......@@ -122,8 +119,6 @@ class PluginBlocktypeInternalmedia extends PluginBlocktype {
'limit' => 5,
'selectone' => true,
'artefacttypes' => array('file'),
'extraselect' => $extraselect,
'extrajoin' => $extrajoin,
'template' => 'artefact:file:artefactchooser-element.tpl',
);
}
......
......@@ -843,7 +843,7 @@ class BlockInstance {
// Get list of allowed artefacts
require_once('view.php');
$searchdata = array(
'extraselect' => 'id IN (' . join(',', array_map('intval', $artefacts)) . ')',
'extraselect' => array(array('fieldname' => 'id', 'type' => 'int', 'values' => $artefacts)),
'userartefactsallowed' => true, // If this is a group view, the user can add personally owned artefacts
);
list($allowed, $count) = View::get_artefactchooser_artefacts(
......
......@@ -1781,7 +1781,7 @@ class View {
safe_require('blocktype', $data['blocktype']);
$blocktypeclass = generate_class_name('blocktype', $data['blocktype']);
$data['sortorder'] = 'title';
$data['sortorder'] = array(array('fieldname' => 'title', 'order' => 'ASC'));
if (method_exists($blocktypeclass, 'artefactchooser_get_sort_order')) {
$data['sortorder'] = call_static_method($blocktypeclass, 'artefactchooser_get_sort_order');
}
......@@ -1876,13 +1876,64 @@ class View {
$offset = !empty($data['offset']) ? $data['offset'] : null;
$limit = !empty($data['limit']) ? $data['limit'] : null;
$sortorder = !empty($data['sortorder']) ? $data['sortorder'] : false;
$extraselect = (isset($data['extraselect']) ? ' AND ' . $data['extraselect'] : '');
$from = ' FROM {artefact} a ';
if (isset($data['extrajoin'])) {
$from .= $data['extrajoin'];
$sortorder = '';
if (!empty($data['sortorder'])) {
foreach ($data['sortorder'] as $field) {
if (!preg_match('/^[a-zA-Z_0-9"]+$/', $field['fieldname'])) {
continue; // skip this item (it fails validation)
}
$order = 'ASC';
if (!empty($field['order']) && ('DESC' == strtoupper($field['order']))) {
$order = 'DESC';
}
if (empty($sortorder)) {
$sortorder .= 'ORDER BY ';
}
else {
$sortorder .= ', ';
}
$sortorder .= $field['fieldname'] . ' ' . $order;
}
}
$extraselect = '';
if (isset($data['extraselect'])) {
foreach ($data['extraselect'] as $field) {
if (!preg_match('/^[a-zA-Z_0-9"]+$/', $field['fieldname'])) {
continue; // skip this item (it fails validation)
}
// Sanitise all values
$values = $field['values'];
foreach ($values as &$val) {
if ($field['type'] == 'int') {
$val = (int)$val;
}
elseif ($field['type'] == 'string') {
$val = db_quote($val);
}
else {
throw new SystemException("Unsupported field type '" . $field['type'] . "' passed to View::get_artefactchooser_artefacts");
}
}
$extraselect .= ' AND ';
if (count($values) > 1) {
$extraselect .= $field['fieldname'] . ' IN (' . implode(', ', $values) . ')';
}
else {
$extraselect .= $field['fieldname'] . ' = ' . reset($values);
}
}
}
$from = ' FROM {artefact} a ';
if ($group) {
// Get group-owned artefacts that the user has view
// permission on, and site-owned artefacts
......@@ -1957,12 +2008,9 @@ class View {
$select .= $extraselect;
$cols = $short ? 'a.id, a.id AS b' : 'a.*'; // get_records_sql_assoc wants > 1 column
if (isset($data['extracols'])) {
$cols .= ', ' . $data['extracols'];
}
$artefacts = get_records_sql_assoc(
'SELECT ' . $cols . $from . ' WHERE ' . $select . ($sortorder ? (' ORDER BY ' . $sortorder) : ''),
'SELECT ' . $cols . $from . ' WHERE ' . $select . $sortorder,
null, $offset, $limit
);
$totalartefacts = count_records_sql('SELECT COUNT(*) ' . $from . ' WHERE ' . $select);
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment