From ad8517ba26cfbf0474d91807e595f6dcc6ac6f3b Mon Sep 17 00:00:00 2001 From: =?utf8?q?Tim=20D=C3=BCsterhus?= Date: Fri, 9 Jul 2021 16:58:42 +0200 Subject: [PATCH] Fix conditions for MultiSelectOptionType This cleans up the SQL conditions used for searching for users with a specific selection and fixes the following issues: - It avoids the use of `escapeString()` in favor of proper prepared statements. - It avoids the use of `preg_quote()` to escape a regular expression for use in MySQL, which might not be safe. - It fixes matching when the options are later reordered, as the saved value is not being normalized and instead reused the order of the options within the select. The generated query does not look great, but is not really worse than the regular expression either. In the future it might be possible to migrate this option type to a JSON based storage and to use `JSON_CONTAINS()`. --- .../option/MultiSelectOptionType.class.php | 54 ++++++++++++------- 1 file changed, 34 insertions(+), 20 deletions(-) diff --git a/wcfsetup/install/files/lib/system/option/MultiSelectOptionType.class.php b/wcfsetup/install/files/lib/system/option/MultiSelectOptionType.class.php index 8b4b4d7d54..a327b4f4ed 100644 --- a/wcfsetup/install/files/lib/system/option/MultiSelectOptionType.class.php +++ b/wcfsetup/install/files/lib/system/option/MultiSelectOptionType.class.php @@ -103,16 +103,23 @@ class MultiSelectOptionType extends SelectOptionType } $value = ArrayUtil::trim($value, false); - $value = \array_map(static function ($value) { - return escapeString(\preg_quote($value)); - }, $value); - - $conditions->add( - "option_value.userOption" . $option->optionID . " REGEXP '" . '(^|\n)' . \implode( - '\n([^\n]*\n)*', - $value - ) . '($|\n)' . "'" - ); + foreach ($value as $entry) { + $escapedEntry = \addcslashes($entry, '%_'); + $conditions->add( + "( + option_value.userOption" . $option->optionID . " LIKE ? + OR option_value.userOption" . $option->optionID . " LIKE ? + OR option_value.userOption" . $option->optionID . " LIKE ? + OR option_value.userOption" . $option->optionID . " = ? + )", + [ + "%\n{$escapedEntry}\n%", + "%\n{$escapedEntry}", + "{$escapedEntry}\n%", + $entry, + ] + ); + } return true; } @@ -127,16 +134,23 @@ class MultiSelectOptionType extends SelectOptionType } $value = ArrayUtil::trim($value, false); - $value = \array_map(static function ($value) { - return escapeString(\preg_quote($value)); - }, $value); - - $userList->getConditionBuilder()->add( - "user_option_value.userOption" . $option->optionID . " REGEXP '" . '(^|\n)' . \implode( - '\n([^\n]*\n)*', - $value - ) . '($|\n)' . "'" - ); + foreach ($value as $entry) { + $escapedEntry = \addcslashes($entry, '%_'); + $userList->getConditionBuilder()->add( + "( + user_option_value.userOption" . $option->optionID . " LIKE ? + OR user_option_value.userOption" . $option->optionID . " LIKE ? + OR user_option_value.userOption" . $option->optionID . " LIKE ? + OR user_option_value.userOption" . $option->optionID . " = ? + )", + [ + "%\n{$escapedEntry}\n%", + "%\n{$escapedEntry}", + "{$escapedEntry}\n%", + $entry, + ] + ); + } } /** -- 2.20.1