From bba528038516b721e8aeee6e87eb53a44f757529 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= <1005065+DeepDiver1975@users.noreply.github.com> Date: Fri, 5 Jun 2026 14:23:31 +0200 Subject: [PATCH 1/3] fix(files_sharing): enforce minimum search length in getUsers() and getGroups() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The admin-configurable user.search_min_length was only enforced in getRemote() via isSearchable() but not in getUsers() or getGroups(). A 1-character search bypassed the intended restriction and allowed authenticated users to enumerate users even when a higher minimum was configured. Add the missing isSearchable() guard to both methods. Signed-off-by: Thomas Müller Signed-off-by: Thomas Müller <1005065+DeepDiver1975@users.noreply.github.com> --- .../lib/Controller/ShareesController.php | 8 +++ apps/files_sharing/tests/API/ShareesTest.php | 65 +++++++++++++++++++ 2 files changed, 73 insertions(+) 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..29c6661cb84b 100644 --- a/apps/files_sharing/tests/API/ShareesTest.php +++ b/apps/files_sharing/tests/API/ShareesTest.php @@ -2084,6 +2084,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]); From 58aae5297690a961703cf34ffa6a2562865133a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= <1005065+DeepDiver1975@users.noreply.github.com> Date: Fri, 5 Jun 2026 15:58:57 +0200 Subject: [PATCH 2/3] chore: add changelog entry for #41580 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Thomas Müller <1005065+DeepDiver1975@users.noreply.github.com> --- changelog/unreleased/41580 | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 changelog/unreleased/41580 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 From 1250cb7303310b7bd121d8410e4b5ea03d92cb47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= <1005065+DeepDiver1975@users.noreply.github.com> Date: Fri, 5 Jun 2026 17:27:00 +0200 Subject: [PATCH 3/3] fix(test): stub isSearchable to return true by default in ShareesTest setUp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Thomas Müller <1005065+DeepDiver1975@users.noreply.github.com> --- apps/files_sharing/tests/API/ShareesTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/apps/files_sharing/tests/API/ShareesTest.php b/apps/files_sharing/tests/API/ShareesTest.php index 29c6661cb84b..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);