From 3df872b9a17ddc3d9a3e25b1b4ebb5cf0f3d843f Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 8 Sep 2022 12:43:37 +0200 Subject: [PATCH 1/2] add a limit to the amount of operators a client can add to a search query Signed-off-by: Robin Appelman --- apps/dav/lib/Files/FileSearchBackend.php | 27 ++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/apps/dav/lib/Files/FileSearchBackend.php b/apps/dav/lib/Files/FileSearchBackend.php index 7ee827798493d..c819fa6afc684 100644 --- a/apps/dav/lib/Files/FileSearchBackend.php +++ b/apps/dav/lib/Files/FileSearchBackend.php @@ -55,6 +55,8 @@ use SearchDAV\Query\Query; class FileSearchBackend implements ISearchBackend { + const OPERATOR_LIMIT = 100; + /** @var CachingTree */ private $tree; @@ -315,6 +317,11 @@ private function transformQuery(Query $query): ISearchQuery { } } + $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 @@ private function transformQuery(Query $query): ISearchQuery { ); } + 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 From 3236a8e2af9383edc258b7fe31cb9291fac4df26 Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Thu, 8 Sep 2022 12:43:50 +0200 Subject: [PATCH 2/2] add test for search operator limit Signed-off-by: Robin Appelman --- .../unit/Files/FileSearchBackendTest.php | 101 +++++++++++++----- 1 file changed, 76 insertions(+), 25 deletions(-) diff --git a/apps/dav/tests/unit/Files/FileSearchBackendTest.php b/apps/dav/tests/unit/Files/FileSearchBackendTest.php index da682a5fb2187..b4245ac181b1a 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 . * */ + namespace OCA\DAV\Tests\Files; use OC\Files\Search\SearchComparison; @@ -44,6 +45,7 @@ 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 @@ public function testSearchFilename() { $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 @@ public function testSearchMimetype() { $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 @@ public function testSearchSize() { $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 @@ public function testSearchMtime() { $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 @@ public function testSearchIsCollection() { $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 @@ public function testSearchInvalidProp() { $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 @@ private function getBasicQuery($type, $property, $value = null) { $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 @@ public function testSearchNonFolder() { ->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 @@ public function testSearchLimitOwnerBasic() { ->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 @@ public function testSearchLimitOwnerNested() { ->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 @@ public function testSearchLimitOwnerNested() { $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); + } }