From dc5cf8221b397b10bccfd317c84486eae4afca10 Mon Sep 17 00:00:00 2001 From: AdamKorcz <44787359+AdamKorcz@users.noreply.github.com> Date: Wed, 9 Sep 2026 15:12:41 +0100 Subject: [PATCH] Limit the length and parentheses nesting depth of a search query (#9277) * Limit the length and parentheses nesting depth of a search query. * Use exception --------- Co-authored-by: Alexandre Alapetite --- app/Models/BooleanSearch.php | 16 ++++++++++++++++ app/Models/Context.php | 1 + app/Models/UserQuery.php | 1 + lib/Minz/BadRequestException.php | 10 ++++++++++ lib/Minz/FrontController.php | 2 ++ p/api/query.php | 8 +++++++- tests/app/Models/BooleanSearchTest.php | 17 +++++++++++++++++ 7 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 lib/Minz/BadRequestException.php diff --git a/app/Models/BooleanSearch.php b/app/Models/BooleanSearch.php index e79d0401f..347a2e224 100644 --- a/app/Models/BooleanSearch.php +++ b/app/Models/BooleanSearch.php @@ -6,6 +6,9 @@ declare(strict_types=1); */ class FreshRSS_BooleanSearch implements \Stringable { + private const MAX_SEARCH_LENGTH = 4096; + private const MAX_PARENTHESES_DEPTH = 32; + private string $raw_input = ''; /** @var list */ private array $searches = []; @@ -15,6 +18,7 @@ class FreshRSS_BooleanSearch implements \Stringable { * @param int $level * @param 'AND'|'OR'|'AND NOT'|'OR NOT' $operator * @param bool $allowUserQueries + * @throws Minz_BadRequestException if the search is too long or if the parentheses are nested too deeply */ public function __construct( string $input, @@ -32,6 +36,9 @@ class FreshRSS_BooleanSearch implements \Stringable { $this->raw_input = $input; if ($level === 0) { + if (strlen($input) > self::MAX_SEARCH_LENGTH) { + throw new Minz_BadRequestException('Search is too long!'); + } $input = self::escapeLiterals($input); if ($expandUserQueries || !$allowUserQueries) { $input = $this->parseUserQueryNames($input, $allowUserQueries); @@ -220,8 +227,13 @@ class FreshRSS_BooleanSearch implements \Stringable { * If the query contains a mix of `OR` expressions with and without parentheses, * then add parentheses to make the query consistent. * Example: '(ab (cd OR ef)) OR gh OR ij OR (kl)' becomes '(ab ((cd) OR (ef))) OR (gh) OR (ij) OR (kl)' + * + * @throws Minz_BadRequestException if the search is too long or if the parentheses are nested too deeply */ public static function consistentOrParentheses(string $input): string { + if (strlen($input) > self::MAX_SEARCH_LENGTH) { + throw new Minz_BadRequestException('Search is too long!'); + } if (!preg_match('/(?= self::MAX_PARENTHESES_DEPTH) { // @phpstan-ignore greaterOrEqual.alwaysFalse + throw new Minz_BadRequestException('Search has too deeply nested parentheses!'); + } $parenthesesCount++; } elseif ($c === ')') { $parenthesesCount--; @@ -586,6 +601,7 @@ class FreshRSS_BooleanSearch implements \Stringable { /** * @param bool $expandUserQueries Whether to expand user queries (saved searches) or not + * @throws Minz_BadRequestException if the search is too long or if the parentheses are nested too deeply */ public function toString(bool $expandUserQueries = true): string { if ($expandUserQueries) { diff --git a/app/Models/Context.php b/app/Models/Context.php index 7b8551704..2eec03f21 100644 --- a/app/Models/Context.php +++ b/app/Models/Context.php @@ -233,6 +233,7 @@ final class FreshRSS_Context { * - next (default: empty string) * - hours (default: 0) * @throws FreshRSS_Context_Exception + * @throws Minz_BadRequestException if the search is too long or if the parentheses are nested too deeply * @throws Minz_ConfigurationNamespaceException * @throws Minz_PDOConnectionException */ diff --git a/app/Models/UserQuery.php b/app/Models/UserQuery.php index f55364aa7..d41adbb85 100644 --- a/app/Models/UserQuery.php +++ b/app/Models/UserQuery.php @@ -50,6 +50,7 @@ class FreshRSS_UserQuery { * publishLabelsInsteadOfTags?:bool,description?:string,imageUrl?:string} $query * @param array $categories * @param array $labels + * @throws Minz_BadRequestException if the search is too long or if the parentheses are nested too deeply */ public function __construct(array $query, array $categories, array $labels) { $this->categories = []; diff --git a/lib/Minz/BadRequestException.php b/lib/Minz/BadRequestException.php new file mode 100644 index 000000000..fb2f7daac --- /dev/null +++ b/lib/Minz/BadRequestException.php @@ -0,0 +1,10 @@ +dispatcher->run(); + } catch (Minz_BadRequestException $e) { + Minz_Error::error(400, ['error' => [$e->getMessage()]], redirect: true); } catch (Minz_Exception $e) { try { Minz_Log::error($e->getMessage()); diff --git a/p/api/query.php b/p/api/query.php index a4bc6f2d4..6b38aad51 100644 --- a/p/api/query.php +++ b/p/api/query.php @@ -100,7 +100,13 @@ foreach (FreshRSS_Context::userConf()->queries as $raw_query) { $search = $query->getSearch()->toString(); // Note: we disallow references to user queries in public user search to avoid sniffing internal user queries - $userSearch = new FreshRSS_BooleanSearch(Minz_Request::paramString('search', plaintext: true), 0, 'AND', allowUserQueries: false); + try { + $userSearch = new FreshRSS_BooleanSearch(Minz_Request::paramString('search', plaintext: true), 0, 'AND', allowUserQueries: false); + } catch (Minz_BadRequestException $e) { + header('HTTP/1.1 400 Bad Request'); + header('Content-Type: text/plain; charset=UTF-8'); + die($e->getMessage()); + } if ($userSearch->toString() !== '') { if ($search === '') { $search = $userSearch->toString(); diff --git a/tests/app/Models/BooleanSearchTest.php b/tests/app/Models/BooleanSearchTest.php index 1d7ebeeb1..6e7005c67 100644 --- a/tests/app/Models/BooleanSearchTest.php +++ b/tests/app/Models/BooleanSearchTest.php @@ -30,4 +30,21 @@ final class BooleanSearchTest extends \PHPUnit\Framework\TestCase { self::assertSame($expectedSql, trim($sql)); self::assertSame($expectedValues, $values); } + + /** @return list */ + public static function provideTooLongOrTooDeepSearches(): array { + $tooLong = str_repeat('ab ', 1400); // Long enough to exceed the maximum search length + $tooDeep = str_repeat('(', 40) . 'ab' . str_repeat(')', 40); // Deeper than the maximum parentheses depth + return [ + [$tooLong], + [$tooDeep], + ]; + } + + #[DataProvider('provideTooLongOrTooDeepSearches')] + public function test_constructor_rejectsTooLongOrTooDeepSearches(string $input): void { + self::expectException(Minz_BadRequestException::class); + // Tests run at the default PHP memory limit; a brute-force 1400-deep search would consume too much memory + new FreshRSS_BooleanSearch($input); + } }