diff --git a/apps/files_sharing/lib/Controller/ShareesController.php b/apps/files_sharing/lib/Controller/ShareesController.php index 7287e83d5120..07cc514822b3 100644 --- a/apps/files_sharing/lib/Controller/ShareesController.php +++ b/apps/files_sharing/lib/Controller/ShareesController.php @@ -170,6 +170,10 @@ protected function getUsers($search) { $this->result['users'] = []; return; } + if (!$this->userSearch->isSearchable($search)) { + $this->result['users'] = []; + return; + } $userGroups = []; if ($this->shareWithGroupOnly || $this->shareeEnumerationGroupMembers) { @@ -310,6 +314,10 @@ protected function getGroups($search) { $this->result['groups'] = []; return; } + if (!$this->userSearch->isSearchable($search)) { + $this->result['groups'] = []; + return; + } $groups = $this->groupManager->search($search, $this->limit, $this->offset, 'sharing'); $groupIds = \array_map(function (IGroup $group) { diff --git a/apps/files_sharing/tests/API/ShareesTest.php b/apps/files_sharing/tests/API/ShareesTest.php index a131131b861d..3e79a820c11b 100644 --- a/apps/files_sharing/tests/API/ShareesTest.php +++ b/apps/files_sharing/tests/API/ShareesTest.php @@ -124,6 +124,8 @@ protected function setUp(): void { $this->userSearch = $this->getMockBuilder(UserSearch::class) ->disableOriginalConstructor() ->getMock(); + // Default: all search terms are acceptable unless a specific test overrides this. + $this->userSearch->method('isSearchable')->willReturn(true); $this->customRemoteSearchMock = $this->createMock(IRemoteShareesSearch::class); @@ -2084,6 +2086,71 @@ public function dataTestFixRemoteUrl() { ]; } + /** + * Verify that a short but non-empty search term is blocked when + * user.search_min_length is configured — the previous code only blocked + * empty strings and allowed single-character enumeration attacks. + */ + public function testGetUsersBlocksShortSearchTerm() { + self::invokePrivate($this->sharees, 'limit', [2]); + self::invokePrivate($this->sharees, 'offset', [0]); + self::invokePrivate($this->sharees, 'shareWithGroupOnly', [false]); + self::invokePrivate($this->sharees, 'shareeEnumeration', [true]); + self::invokePrivate($this->sharees, 'shareeEnumerationGroupMembers', [false]); + + // Simulate admin setting user.search_min_length = 2 + $this->userSearch->expects($this->any()) + ->method('getSearchMinLength') + ->willReturn(2); + $this->userSearch->expects($this->once()) + ->method('isSearchable') + ->with('a') + ->willReturn(false); + + // userManager->find() must NOT be called for a too-short search term + $this->userManager->expects($this->never()) + ->method('find'); + $this->userManager->expects($this->never()) + ->method('get'); + + self::invokePrivate($this->sharees, 'getUsers', ['a']); + $result = self::invokePrivate($this->sharees, 'result'); + + $this->assertEmpty($result['exact']['users'], 'Exact users must be empty for short search'); + $this->assertEmpty($result['users'], 'Users must be empty for short search'); + } + + /** + * Verify that a short but non-empty group search term is blocked when + * user.search_min_length is configured. + */ + public function testGetGroupsBlocksShortSearchTerm() { + self::invokePrivate($this->sharees, 'limit', [2]); + self::invokePrivate($this->sharees, 'offset', [0]); + self::invokePrivate($this->sharees, 'shareWithMembershipGroupOnly', [false]); + self::invokePrivate($this->sharees, 'shareeEnumeration', [true]); + self::invokePrivate($this->sharees, 'shareeEnumerationGroupMembers', [false]); + + // Simulate admin setting user.search_min_length = 2 + $this->userSearch->expects($this->any()) + ->method('getSearchMinLength') + ->willReturn(2); + $this->userSearch->expects($this->once()) + ->method('isSearchable') + ->with('a') + ->willReturn(false); + + // groupManager->search() must NOT be called for a too-short search term + $this->groupManager->expects($this->never()) + ->method('search'); + + self::invokePrivate($this->sharees, 'getGroups', ['a']); + $result = self::invokePrivate($this->sharees, 'result'); + + $this->assertEmpty($result['exact']['groups'], 'Exact groups must be empty for short search'); + $this->assertEmpty($result['groups'], 'Groups must be empty for short search'); + } + public function testGetUserWithSearchAttributes() { self::invokePrivate($this->sharees, 'limit', [2]); self::invokePrivate($this->sharees, 'offset', [0]); diff --git a/changelog/unreleased/41580 b/changelog/unreleased/41580 new file mode 100644 index 000000000000..bc0e0e52ba7e --- /dev/null +++ b/changelog/unreleased/41580 @@ -0,0 +1,9 @@ +Security: Enforce minimum search length server-side in sharees endpoint + +The admin-configured minimum search length was only enforced client-side +and in the remote search path. The user and group search paths only +blocked empty strings, allowing a single-character query to bypass the +configured limit and enumerate users. The missing server-side guard has +been added to both paths. + +https://github.com/owncloud/core/pull/41580