From 0f4e6d54aa7f2492c3e7c64af10a31e87c038533 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elan=20Ruusam=C3=A4e?= Date: Tue, 9 Nov 2021 22:46:51 +0200 Subject: [PATCH 1/5] Revert fix for sort_by query This reverts commit b664d35d2f019fa8ef619d5984c745c2b99fd4f9. This reverts commit efdc34d39a7e6c90996a8cf0318b73adc777dce9. The fix didn't account for custom fields and resulted sql error in case of invalid input, which resulted a 500 page. --- lib/eventum/class.search.php | 9 --------- 1 file changed, 9 deletions(-) diff --git a/lib/eventum/class.search.php b/lib/eventum/class.search.php index 024109c70a..aa4ec9e98f 100644 --- a/lib/eventum/class.search.php +++ b/lib/eventum/class.search.php @@ -371,15 +371,6 @@ public static function getListing($prj_id, array $options, $current_row = 0, $ma $sort_by = Misc::escapeString($options['sort_by']); } - // default sort by option - $default_sort_by_options = ['last_action_date', 'pri_rank', 'iss_id', 'sta_rank', 'iss_summary']; - // check $sort_by - if (in_array($sort_by, $default_sort_by_options, true)) { - $sort_by = $sort_by; - } else { - $sort_by = ''; - } - $stmt .= ' GROUP BY iss_id From c732276e0855db9c0d3bafb33d41484ed34202ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elan=20Ruusam=C3=A4e?= Date: Tue, 9 Nov 2021 22:56:29 +0200 Subject: [PATCH 2/5] Escape $sort_by with quoteIdentifier not with escapeString --- lib/eventum/class.search.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/eventum/class.search.php b/lib/eventum/class.search.php index aa4ec9e98f..50d45f53f4 100644 --- a/lib/eventum/class.search.php +++ b/lib/eventum/class.search.php @@ -368,7 +368,7 @@ public static function getListing($prj_id, array $options, $current_row = 0, $ma $fld_details = Custom_Field::getDetails($fld_id); $sort_by = 'cf_sort.' . Custom_Field::getDBValueFieldNameByType($fld_details['fld_type']); } else { - $sort_by = Misc::escapeString($options['sort_by']); + $sort_by = DB_Helper::getInstance()->quoteIdentifier($options['sort_by']); } $stmt .= ' From 9119c50e86386a4942b6aaf099448df07ad97dbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elan=20Ruusam=C3=A4e?= Date: Wed, 10 Nov 2021 00:22:22 +0200 Subject: [PATCH 3/5] Validate sort_by against a whitelist --- lib/eventum/class.search.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/eventum/class.search.php b/lib/eventum/class.search.php index 50d45f53f4..bbd0631895 100644 --- a/lib/eventum/class.search.php +++ b/lib/eventum/class.search.php @@ -20,6 +20,8 @@ */ class Search { + private const SORT_BY_FIELDS = ['last_action_date', 'pri_rank', 'iss_id', 'sta_rank', 'iss_summary']; + /** * Method used to get a specific parameter in the issue listing cookie. * @@ -368,7 +370,12 @@ public static function getListing($prj_id, array $options, $current_row = 0, $ma $fld_details = Custom_Field::getDetails($fld_id); $sort_by = 'cf_sort.' . Custom_Field::getDBValueFieldNameByType($fld_details['fld_type']); } else { - $sort_by = DB_Helper::getInstance()->quoteIdentifier($options['sort_by']); + if (in_array($options['sort_by'], self::SORT_BY_FIELDS, true)) { + $sort_by = $options['sort_by']; + } else { + $sort_by = 'pri_rank'; + } + $sort_by = DB_Helper::getInstance()->quoteIdentifier($sort_by); } $stmt .= ' From 7e63b60b0b5537edae0390409f128f7df809e9b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elan=20Ruusam=C3=A4e?= Date: Wed, 10 Nov 2021 01:53:38 +0200 Subject: [PATCH 4/5] Validate sort_by early as in saveSearchParams() --- lib/eventum/class.search.php | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/lib/eventum/class.search.php b/lib/eventum/class.search.php index bbd0631895..7bb0c7f548 100644 --- a/lib/eventum/class.search.php +++ b/lib/eventum/class.search.php @@ -20,7 +20,7 @@ */ class Search { - private const SORT_BY_FIELDS = ['last_action_date', 'pri_rank', 'iss_id', 'sta_rank', 'iss_summary']; + private const SORT_BY_FIELDS = ['last_action_date', 'pri_rank', 'iss_id', 'sta_rank', 'iss_summary', 'custom_field']; /** * Method used to get a specific parameter in the issue listing cookie. @@ -89,7 +89,8 @@ public static function saveSearchParams($save_db = true): array { $request_only = !$save_db; // if we should only look at get / post not the DB or cookies - $sort_by = self::getParam('sort_by', $request_only); + $sort_by = self::getParam('sort_by', $request_only, self::SORT_BY_FIELDS); + $sort_by = $sort_by ?: 'pri_rank'; $sort_order = self::getParam('sort_order', $request_only, ['asc', 'desc']); $rows = self::getParam('rows', $request_only); $hide_closed = self::getParam('hide_closed', $request_only); @@ -370,12 +371,7 @@ public static function getListing($prj_id, array $options, $current_row = 0, $ma $fld_details = Custom_Field::getDetails($fld_id); $sort_by = 'cf_sort.' . Custom_Field::getDBValueFieldNameByType($fld_details['fld_type']); } else { - if (in_array($options['sort_by'], self::SORT_BY_FIELDS, true)) { - $sort_by = $options['sort_by']; - } else { - $sort_by = 'pri_rank'; - } - $sort_by = DB_Helper::getInstance()->quoteIdentifier($sort_by); + $sort_by = DB_Helper::getInstance()->quoteIdentifier($options['sort_by']); } $stmt .= ' From 7ba9f197bb9f9d9d1771d1380df0b8d5b0974903 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elan=20Ruusam=C3=A4e?= Date: Wed, 10 Nov 2021 00:29:28 +0200 Subject: [PATCH 5/5] Update changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a33638ba59..27a5d40b48 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ See [Upgrading] for details on how to upgrade. -- Fix `sort_by` not being filtered in search form, #1252 +- Fix `sort_by` not being filtered in search form, #1252, #1255 - Fix bug allowing to execute arbitrary JavaScript in SVG files, #1251 [3.10.8]: https://github.com/eventum/eventum/compare/v3.10.7...master