Explorar o código

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 <alexandre@alapetite.fr>
AdamKorcz hai 13 horas
pai
achega
dc5cf8221b

+ 16 - 0
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<FreshRSS_BooleanSearch|FreshRSS_Search> */
 	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('/(?<!\\\\)\\(/', $input)) {
 			// No unescaped parentheses in the input
 			return trim($input);
@@ -247,6 +259,9 @@ class FreshRSS_BooleanSearch implements \Stringable {
 						}
 						$c = '';
 					}
+					if ($parenthesesCount >= 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) {

+ 1 - 0
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
 	 */

+ 1 - 0
app/Models/UserQuery.php

@@ -50,6 +50,7 @@ class FreshRSS_UserQuery {
 	 * 	publishLabelsInsteadOfTags?:bool,description?:string,imageUrl?:string} $query
 	 * @param array<FreshRSS_Category> $categories
 	 * @param array<FreshRSS_Tag> $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 = [];

+ 10 - 0
lib/Minz/BadRequestException.php

@@ -0,0 +1,10 @@
+<?php
+declare(strict_types=1);
+
+/**
+ * An exception raised when user input is invalid.
+ * The front controller turns it into a HTTP 400 Bad Request error page.
+ */
+class Minz_BadRequestException extends Minz_Exception {
+
+}

+ 2 - 0
lib/Minz/FrontController.php

@@ -62,6 +62,8 @@ class Minz_FrontController {
 	public function run(): void {
 		try {
 			$this->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());

+ 7 - 1
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();

+ 17 - 0
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<list{string}> */
+	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);
+	}
 }