From 56060caa4c44620929b6e17fe4622343750ad302 Mon Sep 17 00:00:00 2001 From: Derky Date: Thu, 14 Mar 2019 21:46:02 +0100 Subject: [PATCH 1/5] [ticket/security/235] Apply wildcard char count patch SECURITY-235 --- phpBB/phpbb/search/fulltext_native.php | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/phpBB/phpbb/search/fulltext_native.php b/phpBB/phpbb/search/fulltext_native.php index 4172e2cc4f..9a6d62f9d8 100644 --- a/phpBB/phpbb/search/fulltext_native.php +++ b/phpBB/phpbb/search/fulltext_native.php @@ -190,7 +190,7 @@ class fulltext_native extends \phpbb\search\base */ public function split_keywords($keywords, $terms) { - $tokens = '+-|()*'; + $tokens = '+-|()* '; $keywords = trim($this->cleanup($keywords, $tokens)); @@ -224,12 +224,10 @@ class fulltext_native extends \phpbb\search\base $keywords[$i] = '|'; break; case '*': - if ($i === 0 || ($keywords[$i - 1] !== '*' && strcspn($keywords[$i - 1], $tokens) === 0)) + // $i can never be 0 here since $open_bracket is initialised to false + if (strpos($tokens, $keywords[$i - 1]) !== false && ($i + 1 === $n || strpos($tokens, $keywords[$i + 1]) !== false)) { - if ($i === $n - 1 || ($keywords[$i + 1] !== '*' && strcspn($keywords[$i + 1], $tokens) === 0)) - { - $keywords = substr($keywords, 0, $i) . substr($keywords, $i + 1); - } + $keywords[$i] = '|'; } break; } @@ -264,7 +262,7 @@ class fulltext_native extends \phpbb\search\base } } - if ($open_bracket) + if ($open_bracket !== false) { $keywords .= ')'; } @@ -409,8 +407,16 @@ class fulltext_native extends \phpbb\search\base { if (strpos($word_part, '*') !== false) { - $id_words[] = '\'' . $this->db->sql_escape(str_replace('*', '%', $word_part)) . '\''; - $non_common_words[] = $word_part; + $len = utf8_strlen(str_replace('*', '', $word_part)); + if ($len >= $this->word_length['min'] && $len <= $this->word_length['max']) + { + $id_words[] = '\'' . $this->db->sql_escape(str_replace('*', '%', $word_part)) . '\''; + $non_common_words[] = $word_part; + } + else + { + $this->common_words[] = $word_part; + } } else if (isset($words[$word_part])) { From fd195fba210c8625e968ef5553e61864747c8d44 Mon Sep 17 00:00:00 2001 From: Derky Date: Thu, 25 Apr 2019 21:51:04 +0200 Subject: [PATCH 2/5] [ticket/security/235] Remove non trailing wildcards from search keywords Database indexes are only used if wildcards are used at the end. SECURITY-235 --- phpBB/phpbb/search/fulltext_native.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/phpBB/phpbb/search/fulltext_native.php b/phpBB/phpbb/search/fulltext_native.php index 9a6d62f9d8..478fe5616d 100644 --- a/phpBB/phpbb/search/fulltext_native.php +++ b/phpBB/phpbb/search/fulltext_native.php @@ -305,6 +305,11 @@ class fulltext_native extends \phpbb\search\base } } + // Remove non trailing wildcards from each word to prevent a full table scan (it's now using the database index) + $match = '#\*(?!$)\b#'; + $replace = '$1'; + $keywords = preg_replace($match, $replace, $keywords); + // set the search_query which is shown to the user $this->search_query = $keywords; From 8a73eb5f0ff912454e6479539f972081e54baa1c Mon Sep 17 00:00:00 2001 From: Derky Date: Fri, 26 Apr 2019 00:52:43 +0200 Subject: [PATCH 3/5] [ticket/security/235] Use whitespace instead of word boundary regex to remove wildcards This fixes removing the wildcard in the following search query: *.test SECURITY-235 --- phpBB/phpbb/search/fulltext_native.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/phpbb/search/fulltext_native.php b/phpBB/phpbb/search/fulltext_native.php index 478fe5616d..1925623b80 100644 --- a/phpBB/phpbb/search/fulltext_native.php +++ b/phpBB/phpbb/search/fulltext_native.php @@ -306,7 +306,7 @@ class fulltext_native extends \phpbb\search\base } // Remove non trailing wildcards from each word to prevent a full table scan (it's now using the database index) - $match = '#\*(?!$)\b#'; + $match = '#\*(?!$|\s)#'; $replace = '$1'; $keywords = preg_replace($match, $replace, $keywords); From da9910850a168f73c6b8dd8407a01f47d27ca1d8 Mon Sep 17 00:00:00 2001 From: Derky Date: Fri, 26 Apr 2019 00:56:48 +0200 Subject: [PATCH 4/5] [ticket/security/235] Only allow one wildcard in the search query to limit the database load SECURITY-235 --- phpBB/phpbb/search/fulltext_native.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/phpBB/phpbb/search/fulltext_native.php b/phpBB/phpbb/search/fulltext_native.php index 1925623b80..c83de75eed 100644 --- a/phpBB/phpbb/search/fulltext_native.php +++ b/phpBB/phpbb/search/fulltext_native.php @@ -310,6 +310,15 @@ class fulltext_native extends \phpbb\search\base $replace = '$1'; $keywords = preg_replace($match, $replace, $keywords); + // Only allow one wildcard in the search query to limit the database load + $match = '#\*#'; + $replace = '$1'; + $count_wildcards = substr_count($keywords, '*'); + + // Reverse the string to remove all wildcards except the first one + $keywords = strrev(preg_replace($match, $replace, strrev($keywords), $count_wildcards - 1)); + unset($count_wildcards); + // set the search_query which is shown to the user $this->search_query = $keywords; From 2353ad11f2afe213e407842a9a9e1533c0de57b0 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Fri, 26 Apr 2019 23:39:51 +0200 Subject: [PATCH 5/5] [ticket/security/235] Update search native tests SECURITY-235 --- tests/search/native_test.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/search/native_test.php b/tests/search/native_test.php index 29d0d0a8d3..0e6f719cef 100644 --- a/tests/search/native_test.php +++ b/tests/search/native_test.php @@ -70,7 +70,7 @@ class phpbb_search_native_test extends phpbb_search_test_case 'ba*az', 'all', true, - array('\'ba%az\''), + array(4), array(), array(), ), @@ -78,7 +78,7 @@ class phpbb_search_native_test extends phpbb_search_test_case 'ba*z', 'all', true, - array('\'ba%z\''), + array(), // <= 3 chars after removing * array(), array(), ), @@ -86,7 +86,7 @@ class phpbb_search_native_test extends phpbb_search_test_case 'baa* baaz*', 'all', true, - array('\'baa%\'', '\'baaz%\''), + array('\'baa%\'', 4), array(), array(), ), @@ -94,7 +94,7 @@ class phpbb_search_native_test extends phpbb_search_test_case 'ba*z baa*', 'all', true, - array('\'ba%z\'', '\'baa%\''), + array('\'baa%\''), // baz is <= 3 chars, only baa* is left array(), array(), ),