Skip to content

Commit 6ebc1c2

Browse files
andiamineandiamine
committed
Fix critical security vulnerabilities across JoomCCK component
- SQL injection: Convert 40+ raw SQL queries to Joomla QueryBuilder with proper quoting ($db->quote(), quoteName(), (int) casting, ArrayHelper::toInteger) across controllers (ajax, cat, items, import, files), models (pack, tfield), tables (record), fields (datetime, textarea, joomcckrelate), and views - Path traversal: Add realpath() validation in ajax controller icons() and loadfieldparams() to prevent directory listing outside JPATH_ROOT - File upload hardening: Add CSRF tokens, auth checks, dangerous extension blocklist, and MIME validation to upload() and mooupload() in files controller; add extension whitelist to usercategory image upload - Access control: Add ACL checks to download() and download_attach() in files controller; add CSRF + permission checks to items bulk operations - XSS prevention: Set Content-Type: application/json on AJAX responses, escape user input in error messages, sanitize Content-Disposition filenames - Input validation: Whitelist ORDER BY columns and table names in dynamic queries, whitelist column names in users_filter, sanitize field_type input - Cleanup: Remove var_dump debug statements, replace direct $_POST/$_REQUEST access with Joomla input API, fix double-execute bug in notifications
1 parent e2bd742 commit 6ebc1c2

18 files changed

Lines changed: 412 additions & 165 deletions

File tree

‎components/com_joomcck/controllers/App.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,6 @@ private function _createMenu($section, $menu_type) {
150150
]);
151151

152152
if (!$et->extension_id) {
153-
var_dump('no ext it');
154153
return;
155154
}
156155

‎components/com_joomcck/controllers/ajax.php‎

Lines changed: 105 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -43,26 +43,36 @@ public function usermention()
4343
}
4444
}
4545

46-
$query = "SELECT name, username FROM `#__users` WHERE username LIKE '%" . $db->escape($input->get('q')) . "%' OR name LIKE '%" . $db->escape(\Joomla\CMS\Factory::getApplication()->input->get('q')) . "%'";
46+
$searchTerm = $db->quote('%' . $db->escape($input->getString('q', ''), true) . '%', false);
47+
$query = $db->getQuery(true)
48+
->select($db->quoteName(['name', 'username']))
49+
->from($db->quoteName('#__users'))
50+
->where($db->quoteName('username') . ' LIKE ' . $searchTerm . ' OR ' . $db->quoteName('name') . ' LIKE ' . $searchTerm);
4751
$db->setQuery($query);
4852
$list = $db->loadObjectList();
4953

5054
$out = array();
5155
foreach($list AS $user)
5256
{
53-
5457
$out[] = $user;
5558
}
5659

60+
header('Content-Type: application/json');
5761
echo json_encode($out);
5862
\Joomla\CMS\Factory::getApplication()->close();
5963
}
6064

6165
public function icons()
6266
{
63-
$dir = JPATH_ROOT . '/' . $this->input->getPath('dir');
67+
$dir = realpath(JPATH_ROOT . '/' . $this->input->getPath('dir'));
6468

6569
$out = array();
70+
if($dir === false || strpos($dir, realpath(JPATH_ROOT)) !== 0)
71+
{
72+
AjaxHelper::send($out);
73+
return;
74+
}
75+
6676
if(is_dir($dir))
6777
{
6878
if($dh = opendir($dir))
@@ -621,16 +631,26 @@ public function bookmark()
621631

622632
public function removeucicon()
623633
{
624-
$file = $this->input->get('file');
625-
$id = $this->input->get('id');
626-
$user = \Joomla\CMS\Factory::getApplication()->getIdentity();
634+
$user = \Joomla\CMS\Factory::getApplication()->getIdentity();
635+
if(!$user->get('id'))
636+
{
637+
AjaxHelper::error(Text::_('AJAX_PLEASELOGIN'));
638+
return;
639+
}
640+
641+
$file = $this->input->getCmd('file');
642+
$id = $this->input->getInt('id');
627643
$fullpath = JPATH_ROOT . '/images/usercategories/' . $user->get('id') . DIRECTORY_SEPARATOR . $file;
628644
if(is_file($fullpath))
629645
{
630646
if(\Joomla\Filesystem\File::delete($fullpath))
631647
{
632648
$db = \Joomla\CMS\Factory::getDbo();
633-
$db->setQuery('UPDATE #__js_res_category_user SET icon = "" WHERE id = ' . $id);
649+
$query = $db->getQuery(true)
650+
->update($db->quoteName('#__js_res_category_user'))
651+
->set($db->quoteName('icon') . ' = ' . $db->quote(''))
652+
->where($db->quoteName('id') . ' = ' . (int)$id);
653+
$db->setQuery($query);
634654
$db->execute();
635655
AjaxHelper::send(1);
636656
}
@@ -644,8 +664,8 @@ public function removeucicon()
644664
public function category_children()
645665
{
646666
$db = \Joomla\CMS\Factory::getDbo();
647-
$parent = $this->input->get('parent');
648-
$section = $this->input->get('section');
667+
$parent = $this->input->getInt('parent');
668+
$section = $this->input->getInt('section');
649669

650670
$db->setQuery(
651671
"SELECT
@@ -662,8 +682,8 @@ public function category_children()
662682
FROM `#__js_res_categories` AS c
663683
LEFT JOIN `#__js_res_sections` AS s ON s.id = c.section_id
664684
WHERE c.published = 1
665-
AND c.section_id = {$section}
666-
AND c.parent_id = {$parent}
685+
AND c.section_id = " . (int)$section . "
686+
AND c.parent_id = " . (int)$parent . "
667687
ORDER BY c.lft ASC
668688
");
669689

@@ -676,6 +696,7 @@ public function category_children()
676696
$cat->path = htmlentities($cat->path, ENT_QUOTES, 'UTF-8');
677697
}
678698

699+
header('Content-Type: application/json');
679700
echo json_encode($categories);
680701
\Joomla\CMS\Factory::getApplication()->close();
681702
}
@@ -691,7 +712,9 @@ public function category_childs()
691712
$category = $cat_model->getItem($this->input->getInt('cat_id'));
692713
$cats_model->section = $category->section_id;
693714
$cats_model->parent_id = $category->id;
694-
$cats_model->order = $this->input->get('order', 'c.lft ASC');
715+
$allowedOrders = ['c.lft ASC', 'c.lft DESC', 'c.title ASC', 'c.title DESC', 'c.id ASC', 'c.id DESC'];
716+
$order = $this->input->getString('order', 'c.lft ASC');
717+
$cats_model->order = in_array($order, $allowedOrders) ? $order : 'c.lft ASC';
695718
$cats_model->levels = 1;
696719
$cats_model->all = 0;
697720
$cats_model->nums = 1;
@@ -724,7 +747,7 @@ public function category_filter()
724747
$sql = $db->getQuery(true);
725748
$sql->select('count(rc.catid) as num, rc.catid');
726749
$sql->from('#__js_res_record_category AS rc');
727-
$sql->where("rc.section_id = {$section->id}");
750+
$sql->where("rc.section_id = " . (int)$section->id);
728751
$sql->group('rc.catid');
729752

730753
if($section->params->get('general.cat_mode'))
@@ -816,9 +839,14 @@ public function users_filter()
816839
$db = \Joomla\CMS\Factory::getDbo();
817840

818841
$sql = $db->getQuery(TRUE);
819-
$sql->select('id, ' . $section->params->get('personalize.author_mode', 'username') . ' AS text');
842+
$authorMode = $section->params->get('personalize.author_mode', 'username');
843+
$allowedColumns = ['username', 'name', 'email'];
844+
if (!in_array($authorMode, $allowedColumns)) {
845+
$authorMode = 'username';
846+
}
847+
$sql->select($db->quoteName('id') . ', ' . $db->quoteName($authorMode) . ' AS text');
820848
$sql->from('#__users');
821-
$sql->where("id IN(SELECT user_id FROM #__js_res_record WHERE section_id = {$section->id})");
849+
$sql->where("id IN(SELECT user_id FROM #__js_res_record WHERE section_id = " . (int)$section->id . ")");
822850

823851
$db->setQuery($sql);
824852

@@ -844,9 +872,10 @@ public function category_records()
844872
$sql->from('#__js_res_record');
845873
$sql->where('published = 1');
846874
$sql->where('hidden = 0');
847-
$sql->where("ctime < " . $db->quote(\Joomla\CMS\Factory::getDate()->toSql()));
848-
$sql->where("(extime = '0000-00-00 00:00:00' OR ISNULL(extime) OR extime > '" . \Joomla\CMS\Factory::getDate()->toSql() . "')");
849-
$sql->where("id IN (SELECT record_id FROM #__js_res_record_category WHERE catid = '{$catid}')");
875+
$now = $db->quote(\Joomla\CMS\Factory::getDate()->toSql());
876+
$sql->where($db->quoteName('ctime') . ' < ' . $now);
877+
$sql->where('(' . $db->quoteName('extime') . ' = ' . $db->quote('0000-00-00 00:00:00') . ' OR ISNULL(' . $db->quoteName('extime') . ') OR ' . $db->quoteName('extime') . ' > ' . $now . ')');
878+
$sql->where($db->quoteName('id') . ' IN (SELECT record_id FROM ' . $db->quoteName('#__js_res_record_category') . ' WHERE ' . $db->quoteName('catid') . ' = ' . (int)$catid . ')');
850879
$db->setQuery($sql, 0, ($limit ? $limit + 1 : 0));
851880
$items = array();
852881
if($recs = $db->loadObjectList())
@@ -990,7 +1019,7 @@ public function field_call()
9901019
$field = $field_table->field_type;
9911020
$func = $this->input->get('func');
9921021
$record_id = $this->input->getInt('record_id');
993-
$params = $_REQUEST;
1022+
$params = $this->input->getArray();
9941023

9951024
if(!$field)
9961025
{
@@ -1112,29 +1141,28 @@ public function mark_notification()
11121141

11131142
public function remove_notification()
11141143
{
1115-
$id = $this->input->getInt('id');
11161144
$section_id = $this->input->getInt('section_id');
11171145

11181146
$user = \Joomla\CMS\Factory::getApplication()->getIdentity();
11191147
$db = \Joomla\CMS\Factory::getDbo();
11201148
$query = $db->getQuery(TRUE);
11211149
$query->delete();
1122-
$query->from('#__js_res_notifications');
1123-
$query->where('user_id = ' . $user->id);
1150+
$query->from($db->quoteName('#__js_res_notifications'));
1151+
$query->where($db->quoteName('user_id') . ' = ' . (int)$user->id);
11241152
if($section_id)
11251153
{
1126-
$query->where('ref_2 = ' . $section_id);
1154+
$query->where($db->quoteName('ref_2') . ' = ' . (int)$section_id);
11271155
}
1128-
if($id != 'all')
1156+
if($this->input->getCmd('id') !== 'all')
11291157
{
1130-
if(!is_array($id))
1158+
$ids = $this->input->get('id', array(), 'array');
1159+
$ids = \Joomla\Utilities\ArrayHelper::toInteger($ids);
1160+
if(!empty($ids))
11311161
{
1132-
settype($id, 'array');
1162+
$query->where($db->quoteName('id') . ' IN (' . implode(', ', $ids) . ')');
11331163
}
1134-
$query->where('id IN (' . implode(', ', $id) . ')');
11351164
}
11361165
$db->setQuery($query);
1137-
$db->execute();
11381166

11391167
try {
11401168
$db->execute();
@@ -1147,8 +1175,8 @@ public function remove_notification()
11471175

11481176
public function remove_notification_by()
11491177
{
1150-
$type = $this->input->get('type');
1151-
$list = $this->input->get('list');
1178+
$type = $this->input->getCmd('type');
1179+
$list = $this->input->get('list', array(), 'array');
11521180
if(!count($list) && $type != 'read')
11531181
{
11541182
echo json_encode(
@@ -1163,52 +1191,57 @@ public function remove_notification_by()
11631191
$db = \Joomla\CMS\Factory::getDbo();
11641192
if($type == 'selected')
11651193
{
1166-
$ids = $list;
1194+
$ids = \Joomla\Utilities\ArrayHelper::toInteger($list);
11671195
}
11681196
else
11691197
{
11701198
$user = \Joomla\CMS\Factory::getApplication()->getIdentity();
11711199
$query = $db->getQuery(TRUE);
1172-
$query->select(' id ');
1173-
$query->from('#__js_res_notifications');
1174-
$query->where(' user_id = ' . $user->id);
1200+
$query->select($db->quoteName('id'));
1201+
$query->from($db->quoteName('#__js_res_notifications'));
1202+
$query->where($db->quoteName('user_id') . ' = ' . (int)$user->id);
11751203
if($section_id)
11761204
{
1177-
$query->where('ref_2 = ' . $section_id);
1205+
$query->where($db->quoteName('ref_2') . ' = ' . (int)$section_id);
11781206
}
11791207

11801208
switch($type)
11811209
{
11821210
case 'event':
1183-
$query->where("type IN ('" . implode("', '", $list) . "')");
1211+
$quotedList = array_map(function($v) use ($db) { return $db->quote($v); }, $list);
1212+
$query->where($db->quoteName('type') . ' IN (' . implode(', ', $quotedList) . ')');
11841213
break;
11851214
case 'record':
1186-
$query->where("ref_1 IN (" . implode(",", $list) . ")");
1215+
$intList = \Joomla\Utilities\ArrayHelper::toInteger($list);
1216+
$query->where($db->quoteName('ref_1') . ' IN (' . implode(',', $intList) . ')');
11871217
break;
11881218
case 'section':
1189-
$query->where("ref_2 IN (" . implode(",", $list) . ")");
1219+
$intList = \Joomla\Utilities\ArrayHelper::toInteger($list);
1220+
$query->where($db->quoteName('ref_2') . ' IN (' . implode(',', $intList) . ')');
11901221
break;
11911222
case 'eventer':
1192-
$query->where("eventer IN (" . implode(",", $list) . ")");
1223+
$intList = \Joomla\Utilities\ArrayHelper::toInteger($list);
1224+
$query->where($db->quoteName('eventer') . ' IN (' . implode(',', $intList) . ')');
11931225
break;
11941226
case 'read':
1195-
$query->where("state_new = 0");
1227+
$query->where($db->quoteName('state_new') . ' = 0');
11961228
break;
11971229
}
11981230

11991231
$db->setQuery($query);
12001232
$ids = $db->loadColumn();
12011233
}
12021234

1203-
if(!count($ids))
1235+
$ids = \Joomla\Utilities\ArrayHelper::toInteger($ids);
1236+
if(empty($ids))
12041237
{
12051238
return;
12061239
}
12071240

12081241
$query = $db->getQuery(TRUE);
12091242
$query->delete();
1210-
$query->from('#__js_res_notifications');
1211-
$query->where("id IN (" . implode(", ", $ids) . ")");
1243+
$query->from($db->quoteName('#__js_res_notifications'));
1244+
$query->where($db->quoteName('id') . ' IN (' . implode(', ', $ids) . ')');
12121245
$db->setQuery($query);
12131246

12141247

@@ -1226,32 +1259,28 @@ public function remove_notification_by()
12261259
public function get_notifications()
12271260
{
12281261
$ids = $this->input->get('exist', array(0), 'array');
1262+
$ids = \Joomla\Utilities\ArrayHelper::toInteger($ids);
12291263
$section_id = $this->input->getInt('section_id');
12301264
$user = \Joomla\CMS\Factory::getApplication()->getIdentity();
12311265
$db = \Joomla\CMS\Factory::getDbo();
12321266
$query = $db->getQuery(TRUE);
12331267

12341268
$query->select('*');
1235-
$query->from('#__js_res_notifications');
1236-
$query->where('user_id = ' . $user->id);
1237-
$query->where('notified = 0');
1269+
$query->from($db->quoteName('#__js_res_notifications'));
1270+
$query->where($db->quoteName('user_id') . ' = ' . (int)$user->id);
1271+
$query->where($db->quoteName('notified') . ' = 0');
12381272
if($section_id)
12391273
{
1240-
if(!is_array($section_id))
1241-
{
1242-
settype($section_id, 'array');
1243-
}
1244-
$query->where('ref_2 IN (' . implode(', ', $section_id) . ')');
1274+
$query->where($db->quoteName('ref_2') . ' = ' . (int)$section_id);
12451275
}
1246-
if(count($ids))
1276+
if(!empty($ids))
12471277
{
1248-
$query->where('id NOT IN (' . implode(', ', $ids) . ')');
1278+
$query->where($db->quoteName('id') . ' NOT IN (' . implode(', ', $ids) . ')');
12491279
}
1250-
$query->order('ctime DESC');
1280+
$query->order($db->quoteName('ctime') . ' DESC');
12511281

1252-
$query .= " LIMIT 0, " . $this->input->get('notiflimit', 5);
1253-
1254-
$db->setQuery($query);
1282+
$limit = $this->input->getInt('notiflimit', 5);
1283+
$db->setQuery($query, 0, $limit);
12551284

12561285

12571286
try{
@@ -1337,14 +1366,25 @@ public function loadcommentparams()
13371366
}
13381367
public function loadfieldparams()
13391368
{
1340-
$xml = JPATH_ROOT.$this->input->getString('dir').DIRECTORY_SEPARATOR.str_replace('.php', '.xml', $this->input->get('value'));
1341-
$xml =\Joomla\Filesystem\Path::clean($xml);
1369+
$dir = $this->input->getString('dir');
1370+
$value = $this->input->getCmd('value');
1371+
$xml = \Joomla\Filesystem\Path::clean(JPATH_ROOT . $dir . DIRECTORY_SEPARATOR . str_replace('.php', '.xml', $value));
1372+
1373+
$resolvedPath = realpath(dirname($xml));
1374+
if($resolvedPath === false || strpos($resolvedPath, realpath(JPATH_ROOT)) !== 0)
1375+
{
1376+
\Joomla\CMS\Factory::getApplication()->close();
1377+
return;
1378+
}
1379+
13421380
$form = '';
1343-
1344-
$segments = explode('/', $this->input->getString('dir'));
1345-
FieldHelper::loadLang($segments[4]);
1381+
1382+
$segments = explode('/', $dir);
1383+
if(isset($segments[4])) {
1384+
FieldHelper::loadLang($segments[4]);
1385+
}
13461386
if(is_file($xml)) {
1347-
$form = MFormHelper::getFieldParams($xml, $this->input->get('fid'), str_replace('.php', '', $this->input->get('value')));
1387+
$form = MFormHelper::getFieldParams($xml, $this->input->get('fid'), str_replace('.php', '', $value));
13481388
}
13491389

13501390
echo $form;
@@ -1369,10 +1409,11 @@ public function loadcommerce()
13691409

13701410
$field = MModelBase::getInstance('TField', 'JoomcckModel')->getItem($this->input->get('fid'));
13711411

1412+
$gateway = preg_replace('/[^a-zA-Z0-9_-]/', '', $gateway);
13721413
$xml = JPATH_ROOT . '/components/com_joomcck/gateways' . DIRECTORY_SEPARATOR . $gateway . DIRECTORY_SEPARATOR . $gateway . '.xml';
13731414
if(! is_file($xml))
13741415
{
1375-
echo "File not found: {$xml}";
1416+
echo 'Gateway configuration not found';
13761417
}
13771418
$out = array();
13781419

0 commit comments

Comments
 (0)