Skip to content

Commit 3a6b148

Browse files
committed
Fix SQL Injection in datagrid advanced filter
1 parent d0aeeaf commit 3a6b148

File tree

8 files changed

+108
-78
lines changed

8 files changed

+108
-78
lines changed

Diff for: system/config/version.ini

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@
22
major = 2
33
minor = 16
44
build = 1
5-
date = 20230613
5+
date = 20230912

Diff for: system/controllers/admin/frontend.php

+12-4
Original file line numberDiff line numberDiff line change
@@ -707,11 +707,19 @@ public function getControllersFilesEvents() {
707707

708708
foreach ($controllers as $controller_name) {
709709

710-
$ctrl_file = $this->cms_config->root_path . 'system/controllers/'.$controller_name.'/frontend.php';
711-
if(!is_readable($ctrl_file)){ continue; }
710+
if (!cmsController::enabled($controller_name)) {
711+
continue;
712+
}
713+
714+
$ctrl_file = $this->cms_config->root_path . 'system/controllers/' . $controller_name . '/frontend.php';
715+
if (!is_readable($ctrl_file)) {
716+
continue;
717+
}
712718

713719
$hooks = cmsCore::getFilesList('system/controllers/' . $controller_name . '/hooks', '*.php', true, true);
714-
if (!$hooks) { continue; }
720+
if (!$hooks) {
721+
continue;
722+
}
715723

716724
$controller_object = cmsCore::getController($controller_name);
717725

@@ -724,7 +732,7 @@ public function getControllersFilesEvents() {
724732
// Некоторые хуки не требуют регистрации в базе данных,
725733
// Например, хуки для CRON или иные, которые вызываются напрямую
726734
// Свойство $disallow_event_db_register в классе хука регулирует это поведение
727-
if(empty($hook_object->disallow_event_db_register)){
735+
if (empty($hook_object->disallow_event_db_register)) {
728736

729737
$events[$controller_name][$index] = $event_name;
730738

Diff for: system/controllers/moderation/backend/actions/logs.php

+39-53
Original file line numberDiff line numberDiff line change
@@ -2,51 +2,50 @@
22

33
class actionModerationLogs extends cmsAction {
44

5-
public function run($target_controller = null, $target_subject = null, $target_id = null, $moderator_id = null){
5+
public function run($target_controller = null, $target_subject = null, $target_id = null, $moderator_id = null) {
66

77
cmsCore::loadAllControllersLanguages();
88

99
$grid = $this->loadDataGrid('logs');
1010

11-
$url = href_to($this->root_url, 'logs');
12-
$sub_url = array();
13-
$url_query = array();
14-
$additional_h1 = array();
11+
$url = href_to($this->root_url, 'logs');
12+
$sub_url = [];
13+
$url_query = [];
14+
$additional_h1 = [];
1515
$subj_controller = null;
1616

17-
$action = $this->request->get('action', -1);
17+
$action = $this->request->get('action', -1);
1818
$only_to_delete = $this->request->get('only_to_delete', 0);
19-
if($action > -1){
19+
20+
if ($action > -1) {
2021

2122
$this->model->filterEqual('action', $action);
2223

23-
if($only_to_delete){
24+
if ($only_to_delete) {
2425

2526
$this->model->filterNotNull('date_expired');
2627

2728
$url_query['only_to_delete'] = $only_to_delete;
28-
2929
}
3030

31-
$additional_h1[] = string_lang('LANG_MODERATION_ACTION_'.$action);
31+
$additional_h1[] = string_lang('LANG_MODERATION_ACTION_' . $action);
3232

3333
$url_query['action'] = $action;
34-
3534
}
3635

37-
if(!empty($target_controller)){
36+
if (!empty($target_controller)) {
3837

3938
$subj_controller = cmsCore::getController($target_controller);
4039

4140
$this->model->filterEqual('target_controller', $target_controller);
4241

4342
$sub_url[] = $target_controller;
4443

45-
if(!empty($target_subject)){
44+
if (!empty($target_subject)) {
4645

4746
$ctype = $subj_controller->getContentTypeForModeration($target_subject);
4847

49-
if($ctype){
48+
if ($ctype) {
5049
$target_subject = $ctype['name'];
5150
} else {
5251
return cmsCore::error404();
@@ -56,45 +55,42 @@ public function run($target_controller = null, $target_subject = null, $target_i
5655

5756
$sub_url[] = $target_subject;
5857

59-
if($ctype){
58+
if ($ctype) {
6059
$additional_h1[] = $ctype['title'];
6160
}
6261

63-
if(!empty($target_id)){
62+
if (!empty($target_id)) {
6463

6564
$this->model->filterEqual('target_id', $target_id);
6665

6766
$sub_url[] = $target_id;
6867

6968
$this->model->lockFilters();
7069

71-
$item = $this->model->getItem('moderators_logs', function ($item, $model){
72-
$item['data'] = cmsModel::yamlToArray($item['data']);
73-
return $item;
74-
});
70+
$item = $this->model->getItem('moderators_logs', function ($item, $model) {
71+
$item['data'] = cmsModel::yamlToArray($item['data']);
72+
return $item;
73+
});
7574

76-
if($item){
77-
$additional_h1[] = $item['data']['title'];
78-
}
75+
if ($item) {
76+
$additional_h1[] = $item['data']['title'];
77+
}
7978

8079
$this->model->unlockFilters();
81-
8280
}
83-
8481
}
85-
8682
}
8783

88-
if(!empty($moderator_id)){
84+
if (!empty($moderator_id)) {
8985

9086
$this->model->filterEqual('moderator_id', $moderator_id);
9187

92-
if(count($sub_url) == 3){
88+
if (count($sub_url) == 3) {
9389
$sub_url[] = $moderator_id;
94-
} elseif(count($sub_url) == 2){
90+
} elseif (count($sub_url) == 2) {
9591
$sub_url[] = 0;
9692
$sub_url[] = $moderator_id;
97-
} elseif(count($sub_url) == 1){
93+
} elseif (count($sub_url) == 1) {
9894
$sub_url[] = 0;
9995
$sub_url[] = 0;
10096
$sub_url[] = $moderator_id;
@@ -107,67 +103,57 @@ public function run($target_controller = null, $target_subject = null, $target_i
107103

108104
$user = cmsCore::getModel('users')->getuser($moderator_id);
109105

110-
if($user){
106+
if ($user) {
111107
$additional_h1[] = $user['nickname'];
112108
}
113-
114109
}
115110

116111
if ($this->request->isAjax()) {
117112

118-
$filter = array();
113+
$filter = [];
119114
$filter_str = $this->request->get('filter', '');
120115

121-
if ($filter_str){
116+
if ($filter_str) {
122117
parse_str($filter_str, $filter);
123-
$this->model->applyGridFilter($grid, $filter);
118+
$grid->applyGridFilter($this->model, $filter, 'moderators_logs');
124119
}
125120

126121
$total = $this->model->getCount('moderators_logs');
127-
$perpage = isset($filter['perpage']) ? $filter['perpage'] : admin::perpage;
128-
$pages = ceil($total / $perpage);
129122

130123
$this->model->joinUserLeft('moderator_id');
131124

132-
$data = $this->model->get('moderators_logs', function ($item, $model) use($subj_controller){
125+
$data = $this->model->get('moderators_logs', function ($item, $model) use ($subj_controller) {
133126

134127
$item['data'] = cmsModel::yamlToArray($item['data']);
135128

136-
$item['controller_title'] = string_lang($item['target_controller'].'_CONTROLLER');
129+
$item['controller_title'] = string_lang($item['target_controller'] . '_CONTROLLER');
137130

138131
$item['subject_title'] = $item['controller_title'];
139132

140-
if($subj_controller !== null){
133+
if ($subj_controller !== null) {
141134

142135
$ctype = $subj_controller->getContentTypeForModeration($item['target_subject']);
143136

144137
$item['subject_title'] = $ctype['title'];
145-
146138
}
147139

148140
return $item;
141+
}) ?: [];
149142

150-
});
151-
152-
$this->cms_template->renderGridRowsJSON($grid, $data, $total, $pages);
153-
154-
$this->halt();
155-
143+
return $this->cms_template->renderJSON($grid->makeGridRows($data, $total));
156144
}
157145

158-
if($additional_h1){
146+
if ($additional_h1) {
159147
$this->cms_template->setPageH1($additional_h1);
160148
}
161149

162150
$this->model->resetFilters();
163151

164-
return $this->cms_template->render('backend/logs', array(
152+
return $this->cms_template->render('backend/logs', [
165153
'grid' => $grid,
166154
'sub_url' => $sub_url,
167155
'url_query' => $url_query,
168-
'url' => $url.($sub_url ? '/'.implode('/', $sub_url) : '').(($action > -1) ? '?'.http_build_query($url_query) : '')
169-
));
170-
156+
'url' => $url . ($sub_url ? '/' . implode('/', $sub_url) : '') . (($action > -1) ? '?' . http_build_query($url_query) : '')
157+
]);
171158
}
172-
173159
}

Diff for: system/core/grid.php

+9-3
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ private function load() {
239239

240240
foreach ($grid as $key => $data) {
241241

242-
$this->grid[$key] = array_merge(($this->grid[$key] ?? []), $data);
242+
$this->grid[$key] = is_array($data) ? array_merge(($this->grid[$key] ?? []), $data) : $data;
243243
}
244244

245245
// Фильтр по умолчанию
@@ -429,9 +429,10 @@ public function addToFilter($filter) {
429429
*
430430
* @param cmsModel $model Объект модели, где выбираем записи
431431
* @param array $filter Массив фильтрации
432+
* @param array $table_name Таблица, где ищем поля фильтра
432433
* @return cmsModel
433434
*/
434-
public function applyGridFilter(cmsModel $model, $filter) {
435+
public function applyGridFilter(cmsModel $model, $filter, $table_name) {
435436

436437
// применяем сортировку
437438
if (!empty($filter['order_by']) && !empty($filter['order_to'])) {
@@ -539,9 +540,14 @@ public function applyGridFilter(cmsModel $model, $filter) {
539540
// Дополнительный фильтр
540541
if (!empty($filter['advanced_filter']) && is_string($filter['advanced_filter'])) {
541542

543+
$dataset_filters = [];
544+
542545
parse_str($filter['advanced_filter'], $dataset_filters);
543546

544-
$model->applyDatasetFilters($dataset_filters);
547+
if (!$model->applyDatasetFilters($dataset_filters, true, [], $table_name)) {
548+
549+
$this->grid['filter']['advanced_filter'] = '';
550+
}
545551
}
546552

547553
return $model;

Diff for: system/core/model.php

+44-14
Original file line numberDiff line numberDiff line change
@@ -1196,37 +1196,59 @@ public function filterOnlineUsers() {
11961196
return $this->filterNotNull('online.user_id')->filterTimestampYounger('online.date_created', cmsUser::USER_ONLINE_INTERVAL, 'SECOND');
11971197
}
11981198

1199-
public function applyDatasetFilters($dataset, $ignore_sorting = false, $allowed_fields = []) {
1199+
/**
1200+
* Применяет набор фильтров из массива
1201+
*
1202+
* @param array $dataset Массив фильтров/сортировки
1203+
* @param boolean $only_filters Применять только фильтры
1204+
* @param array $allowed_fields Разрешённые поля для фильтрации
1205+
* @param string $table_name Имя таблицы: если передано, все поля проверяются на доступность в ней
1206+
* @return boolean true, если что-либо применилось, false, если ничего
1207+
*/
1208+
public function applyDatasetFilters($dataset, $only_filters = false, $allowed_fields = [], $table_name = '') {
12001209

1201-
if (!empty($dataset['filters'])) {
1210+
$success = false;
1211+
1212+
if (!empty($dataset['filters']) && is_array($dataset['filters'])) {
12021213

12031214
foreach ($dataset['filters'] as $filter) {
12041215

1216+
// Небольшая валидация
1217+
if (
1218+
empty($filter['field']) || !is_string($filter['field']) ||
1219+
empty($filter['condition'] || !is_string($filter['condition'])) ||
1220+
!array_key_exists('value', $filter) || (!is_string($filter['value']) && $filter['condition'] !== 'in')
1221+
) {
1222+
continue;
1223+
}
1224+
1225+
// Есть ли такое поле в таблице
1226+
if ($table_name && !$this->db->isFieldExists($table_name, $filter['field'])) {
1227+
continue;
1228+
}
1229+
12051230
// Если заданы разрешенные поля, проверяем
12061231
// валидация
12071232
if ($allowed_fields && !in_array($filter['field'], $allowed_fields, true)) {
12081233
continue;
12091234
}
12101235

1211-
if (isset($filter['callback']) && is_callable($filter['callback'])) {
1236+
// Таблица передаётся, когда field в $dataset могут быть переданы любые
1237+
if (!$table_name && isset($filter['callback']) && is_callable($filter['callback'])) {
12121238
$filter['callback']($this, $dataset);
12131239
continue;
12141240
}
12151241

1216-
if (!isset($filter['value'])) {
1217-
continue;
1218-
}
12191242
if (($filter['value'] === '') && !in_array($filter['condition'], ['nn', 'ni'])) {
12201243
continue;
12211244
}
1222-
if (empty($filter['condition'])) {
1223-
continue;
1224-
}
12251245

12261246
if ($filter['value'] !== '' && !is_array($filter['value'])) {
12271247
$filter['value'] = string_replace_user_properties($filter['value']);
12281248
}
12291249

1250+
$success = true;
1251+
12301252
switch ($filter['condition']) {
12311253

12321254
// общие условия
@@ -1272,15 +1294,21 @@ public function applyDatasetFilters($dataset, $ignore_sorting = false, $allowed_
12721294
}
12731295
}
12741296

1275-
if (!empty($dataset['sorting']) && !$ignore_sorting) {
1297+
if (!empty($dataset['sorting']) && !$only_filters) {
1298+
1299+
$success = true;
1300+
12761301
$this->orderByList($dataset['sorting']);
12771302
}
12781303

1279-
if (!empty($dataset['index'])) {
1304+
if (!empty($dataset['index']) && !$only_filters) {
1305+
1306+
$success = true;
1307+
12801308
$this->forceIndex($dataset['index'], 2);
12811309
}
12821310

1283-
return true;
1311+
return $success;
12841312
}
12851313

12861314
/**
@@ -2251,14 +2279,16 @@ public function reorderByList($table_name, $list, $additional_fields = [], $fiel
22512279
/**
22522280
* Применяет к модели фильтры, переданные из просмотра
22532281
* таблицы со списком записей
2282+
* Метод совместимости, не используйте его
22542283
*
22552284
* @param cmsGrid $grid Объект грида
22562285
* @param array $filter
2286+
* @param string $table_name
22572287
* @return $this
22582288
*/
2259-
public function applyGridFilter(cmsGrid $grid, $filter) {
2289+
public function applyGridFilter(cmsGrid $grid, $filter, $table_name = '') {
22602290

2261-
$grid->applyGridFilter($this, $filter);
2291+
$grid->applyGridFilter($this, $filter, $table_name);
22622292

22632293
return $this;
22642294
}

0 commit comments

Comments
 (0)