Merge pull request #33964 from nextcloud/search-limit-operators

add a limit to the amount of operators a client can add to a search query

2 files changed, 103 insertions, 25 deletions

diff --git a/apps/dav/lib/Files/FileSearchBackend.php b/apps/dav/lib/Files/FileSearchBackend.php
index 7ee82779849..c819fa6afc6 100644
--- a/apps/dav/lib/Files/FileSearchBackend.php
+++ b/apps/dav/lib/Files/FileSearchBackend.php
@@ -55,6 +55,8 @@ use SearchDAV\Query\Order;
 use SearchDAV\Query\Query;
 
 class FileSearchBackend implements ISearchBackend {
+	const OPERATOR_LIMIT = 100;
+
 	/** @var CachingTree */
 	private $tree;
 
@@ -315,6 +317,11 @@ class FileSearchBackend implements ISearchBackend {
 			}
 		}
 
+		$operatorCount = $this->countSearchOperators($query->where);
+		if ($operatorCount > self::OPERATOR_LIMIT) {
+			throw new \InvalidArgumentException('Invalid search query, maximum operator limit of ' . self::OPERATOR_LIMIT . ' exceeded, got ' . $operatorCount . ' operators');
+		}
+
 		return new SearchQuery(
 			$this->transformSearchOperation($query->where),
 			(int)$limit->maxResults,
@@ -325,6 +332,26 @@ class FileSearchBackend implements ISearchBackend {
 		);
 	}
 
+	private function countSearchOperators(Operator $operator): int {
+		switch ($operator->type) {
+			case Operator::OPERATION_AND:
+			case Operator::OPERATION_OR:
+			case Operator::OPERATION_NOT:
+				/** @var Operator[] $arguments */
+				$arguments = $operator->arguments;
+				return array_sum(array_map([$this, 'countSearchOperators'], $arguments));
+			case Operator::OPERATION_EQUAL:
+			case Operator::OPERATION_GREATER_OR_EQUAL_THAN:
+			case Operator::OPERATION_GREATER_THAN:
+			case Operator::OPERATION_LESS_OR_EQUAL_THAN:
+			case Operator::OPERATION_LESS_THAN:
+			case Operator::OPERATION_IS_LIKE:
+			case Operator::OPERATION_IS_COLLECTION:
+			default:
+				return 1;
+		}
+	}
+
 	/**
 	 * @param Order $order
 	 * @return ISearchOrder
diff --git a/apps/dav/tests/unit/Files/FileSearchBackendTest.php b/apps/dav/tests/unit/Files/FileSearchBackendTest.php
index da682a5fb21..b4245ac181b 100644
--- a/apps/dav/tests/unit/Files/FileSearchBackendTest.php
+++ b/apps/dav/tests/unit/Files/FileSearchBackendTest.php
@@ -24,6 +24,7 @@
  * along with this program. If not, see <http://www.gnu.org/licenses/>.
  *
  */
+
 namespace OCA\DAV\Tests\Files;
 
 use OC\Files\Search\SearchComparison;
@@ -44,6 +45,7 @@ use OCP\IUser;
 use OCP\Share\IManager;
 use SearchDAV\Backend\SearchPropertyDefinition;
 use SearchDAV\Query\Limit;
+use SearchDAV\Query\Operator;
 use SearchDAV\Query\Query;
 use Test\TestCase;
 
@@ -132,10 +134,10 @@ class FileSearchBackendTest extends TestCase {
 				$this->user
 			))
 			->willReturn([
-				new \OC\Files\Node\Folder($this->rootFolder, $this->view, '/test/path')
+				new \OC\Files\Node\Folder($this->rootFolder, $this->view, '/test/path'),
 			]);
 
-		$query = $this->getBasicQuery(\SearchDAV\Query\Operator::OPERATION_EQUAL, '{DAV:}displayname', 'foo');
+		$query = $this->getBasicQuery(Operator::OPERATION_EQUAL, '{DAV:}displayname', 'foo');
 		$result = $this->search->search($query);
 
 		$this->assertCount(1, $result);
@@ -161,10 +163,10 @@ class FileSearchBackendTest extends TestCase {
 				$this->user
 			))
 			->willReturn([
-				new \OC\Files\Node\Folder($this->rootFolder, $this->view, '/test/path')
+				new \OC\Files\Node\Folder($this->rootFolder, $this->view, '/test/path'),
 			]);
 
-		$query = $this->getBasicQuery(\SearchDAV\Query\Operator::OPERATION_EQUAL, '{DAV:}getcontenttype', 'foo');
+		$query = $this->getBasicQuery(Operator::OPERATION_EQUAL, '{DAV:}getcontenttype', 'foo');
 		$result = $this->search->search($query);
 
 		$this->assertCount(1, $result);
@@ -190,10 +192,10 @@ class FileSearchBackendTest extends TestCase {
 				$this->user
 			))
 			->willReturn([
-				new \OC\Files\Node\Folder($this->rootFolder, $this->view, '/test/path')
+				new \OC\Files\Node\Folder($this->rootFolder, $this->view, '/test/path'),
 			]);
 
-		$query = $this->getBasicQuery(\SearchDAV\Query\Operator::OPERATION_GREATER_THAN, FilesPlugin::SIZE_PROPERTYNAME, 10);
+		$query = $this->getBasicQuery(Operator::OPERATION_GREATER_THAN, FilesPlugin::SIZE_PROPERTYNAME, 10);
 		$result = $this->search->search($query);
 
 		$this->assertCount(1, $result);
@@ -219,10 +221,10 @@ class FileSearchBackendTest extends TestCase {
 				$this->user
 			))
 			->willReturn([
-				new \OC\Files\Node\Folder($this->rootFolder, $this->view, '/test/path')
+				new \OC\Files\Node\Folder($this->rootFolder, $this->view, '/test/path'),
 			]);
 
-		$query = $this->getBasicQuery(\SearchDAV\Query\Operator::OPERATION_GREATER_THAN, '{DAV:}getlastmodified', 10);
+		$query = $this->getBasicQuery(Operator::OPERATION_GREATER_THAN, '{DAV:}getlastmodified', 10);
 		$result = $this->search->search($query);
 
 		$this->assertCount(1, $result);
@@ -248,10 +250,10 @@ class FileSearchBackendTest extends TestCase {
 				$this->user
 			))
 			->willReturn([
-				new \OC\Files\Node\Folder($this->rootFolder, $this->view, '/test/path')
+				new \OC\Files\Node\Folder($this->rootFolder, $this->view, '/test/path'),
 			]);
 
-		$query = $this->getBasicQuery(\SearchDAV\Query\Operator::OPERATION_IS_COLLECTION, 'yes');
+		$query = $this->getBasicQuery(Operator::OPERATION_IS_COLLECTION, 'yes');
 		$result = $this->search->search($query);
 
 		$this->assertCount(1, $result);
@@ -269,7 +271,7 @@ class FileSearchBackendTest extends TestCase {
 		$this->searchFolder->expects($this->never())
 			->method('search');
 
-		$query = $this->getBasicQuery(\SearchDAV\Query\Operator::OPERATION_EQUAL, '{DAV:}getetag', 'foo');
+		$query = $this->getBasicQuery(Operator::OPERATION_EQUAL, '{DAV:}getetag', 'foo');
 		$this->search->search($query);
 	}
 
@@ -280,12 +282,12 @@ class FileSearchBackendTest extends TestCase {
 		$orderBy = [];
 		$select = [];
 		if (is_null($value)) {
-			$where = new \SearchDAV\Query\Operator(
+			$where = new Operator(
 				$type,
 				[new \SearchDAV\Query\Literal($property)]
 			);
 		} else {
-			$where = new \SearchDAV\Query\Operator(
+			$where = new Operator(
 				$type,
 				[new SearchPropertyDefinition($property, true, true, true), new \SearchDAV\Query\Literal($value)]
 			);
@@ -305,7 +307,7 @@ class FileSearchBackendTest extends TestCase {
 			->method('getNodeForPath')
 			->willReturn($davNode);
 
-		$query = $this->getBasicQuery(\SearchDAV\Query\Operator::OPERATION_EQUAL, '{DAV:}displayname', 'foo');
+		$query = $this->getBasicQuery(Operator::OPERATION_EQUAL, '{DAV:}displayname', 'foo');
 		$this->search->search($query);
 	}
 
@@ -321,11 +323,11 @@ class FileSearchBackendTest extends TestCase {
 			->willReturnCallback(function ($query) use (&$receivedQuery) {
 				$receivedQuery = $query;
 				return [
-					new \OC\Files\Node\Folder($this->rootFolder, $this->view, '/test/path')
+					new \OC\Files\Node\Folder($this->rootFolder, $this->view, '/test/path'),
 				];
 			});
 
-		$query = $this->getBasicQuery(\SearchDAV\Query\Operator::OPERATION_EQUAL, FilesPlugin::OWNER_ID_PROPERTYNAME, $this->user->getUID());
+		$query = $this->getBasicQuery(Operator::OPERATION_EQUAL, FilesPlugin::OWNER_ID_PROPERTYNAME, $this->user->getUID());
 		$this->search->search($query);
 
 		$this->assertNotNull($receivedQuery);
@@ -350,22 +352,22 @@ class FileSearchBackendTest extends TestCase {
 			->willReturnCallback(function ($query) use (&$receivedQuery) {
 				$receivedQuery = $query;
 				return [
-					new \OC\Files\Node\Folder($this->rootFolder, $this->view, '/test/path')
+					new \OC\Files\Node\Folder($this->rootFolder, $this->view, '/test/path'),
 				];
 			});
 
-		$query = $this->getBasicQuery(\SearchDAV\Query\Operator::OPERATION_EQUAL, FilesPlugin::OWNER_ID_PROPERTYNAME, $this->user->getUID());
-		$query->where = new \SearchDAV\Query\Operator(
-			\SearchDAV\Query\Operator::OPERATION_AND,
+		$query = $this->getBasicQuery(Operator::OPERATION_EQUAL, FilesPlugin::OWNER_ID_PROPERTYNAME, $this->user->getUID());
+		$query->where = new Operator(
+			Operator::OPERATION_AND,
 			[
-				new \SearchDAV\Query\Operator(
-					\SearchDAV\Query\Operator::OPERATION_EQUAL,
+				new Operator(
+					Operator::OPERATION_EQUAL,
 					[new SearchPropertyDefinition('{DAV:}getcontenttype', true, true, true), new \SearchDAV\Query\Literal('image/png')]
 				),
-				new \SearchDAV\Query\Operator(
-					\SearchDAV\Query\Operator::OPERATION_EQUAL,
+				new Operator(
+					Operator::OPERATION_EQUAL,
 					[new SearchPropertyDefinition(FilesPlugin::OWNER_ID_PROPERTYNAME, true, true, true), new \SearchDAV\Query\Literal($this->user->getUID())]
-				)
+				),
 			]
 		);
 		$this->search->search($query);
@@ -385,4 +387,53 @@ class FileSearchBackendTest extends TestCase {
 		$this->assertEquals(ISearchBinaryOperator::OPERATOR_AND, $operator->getType());
 		$this->assertEmpty($operator->getArguments());
 	}
+
+	public function testSearchOperatorLimit() {
+		$this->tree->expects($this->any())
+			->method('getNodeForPath')
+			->willReturn($this->davFolder);
+
+		$innerOperator = new Operator(
+			Operator::OPERATION_EQUAL,
+			[new SearchPropertyDefinition('{DAV:}getcontenttype', true, true, true), new \SearchDAV\Query\Literal('image/png')]
+		);
+		// 5 child operators
+		$level1Operator = new Operator(
+			Operator::OPERATION_AND,
+			[
+				$innerOperator,
+				$innerOperator,
+				$innerOperator,
+				$innerOperator,
+				$innerOperator,
+			]
+		);
+		// 5^2 = 25 child operators
+		$level2Operator = new Operator(
+			Operator::OPERATION_AND,
+			[
+				$level1Operator,
+				$level1Operator,
+				$level1Operator,
+				$level1Operator,
+				$level1Operator,
+			]
+		);
+		// 5^3 = 125 child operators
+		$level3Operator = new Operator(
+			Operator::OPERATION_AND,
+			[
+				$level2Operator,
+				$level2Operator,
+				$level2Operator,
+				$level2Operator,
+				$level2Operator,
+			]
+		);
+
+		$query = $this->getBasicQuery(Operator::OPERATION_EQUAL, FilesPlugin::OWNER_ID_PROPERTYNAME, $this->user->getUID());
+		$query->where = $level3Operator;
+		$this->expectException(\InvalidArgumentException::class);
+		$this->search->search($query);
+	}
 }