From c3774dea13eb432c61ad26b5ad7c9eeb21876001 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= <1005065+DeepDiver1975@users.noreply.github.com> Date: Mon, 18 May 2026 18:16:56 +0200 Subject: [PATCH 1/4] fix(security): restrict AppConfigController read methods to full admins only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OC10-75: Subadmins could read all oc_appconfig values including SMTP passwords and LDAP credentials via getApps/getKeys/getValue endpoints. Remove @NoAdminRequired so AdminMiddleware enforces full-admin-only access, consistent with the write methods. CVSS: 7.7 Signed-off-by: Thomas Müller <1005065+DeepDiver1975@users.noreply.github.com> --- settings/Controller/AppConfigController.php | 10 +--------- .../Controller/AppConfigControllerTest.php | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/settings/Controller/AppConfigController.php b/settings/Controller/AppConfigController.php index 0e8da029fba..b96339fb5cd 100644 --- a/settings/Controller/AppConfigController.php +++ b/settings/Controller/AppConfigController.php @@ -27,9 +27,7 @@ /** * The code is mostly copied from core/ajax/appconfig.php - * Read methods (getApps, getKeys and getValue) are available to subadmins, - * which wasn't possible with the core/ajax/appconfig.php file. The rest of - * the methods require admin privileges. + * All methods require full admin privileges. * Note that the "hasKey" method is missing. You can do the same in a lot of * cases by trying to get the value of the key. * @@ -54,8 +52,6 @@ public function __construct( } /** - * @NoAdminRequired - * * Get the list of apps */ public function getApps() { @@ -63,8 +59,6 @@ public function getApps() { } /** - * @NoAdminRequired - * * Get the list of keys for that particular app * @param string $app */ @@ -73,8 +67,6 @@ public function getKeys($app) { } /** - * @NoAdminRequired - * * Get the value of the key for that app, or the default value provided * if it's missing. * @param string $app diff --git a/tests/Settings/Controller/AppConfigControllerTest.php b/tests/Settings/Controller/AppConfigControllerTest.php index 26982b653a2..39fd4b0ed4e 100644 --- a/tests/Settings/Controller/AppConfigControllerTest.php +++ b/tests/Settings/Controller/AppConfigControllerTest.php @@ -156,4 +156,22 @@ public function testDeleteAppWrong(): void { $this->assertEquals([], $response->getData()); $this->assertSame(Http::STATUS_BAD_REQUEST, $response->getStatus()); } + + public function testGetAppsRequiresAdmin(): void { + $reflection = new \ReflectionMethod(AppConfigController::class, 'getApps'); + $docComment = (string)$reflection->getDocComment(); + $this->assertStringNotContainsString('@NoAdminRequired', $docComment); + } + + public function testGetKeysRequiresAdmin(): void { + $reflection = new \ReflectionMethod(AppConfigController::class, 'getKeys'); + $docComment = (string)$reflection->getDocComment(); + $this->assertStringNotContainsString('@NoAdminRequired', $docComment); + } + + public function testGetValueRequiresAdmin(): void { + $reflection = new \ReflectionMethod(AppConfigController::class, 'getValue'); + $docComment = (string)$reflection->getDocComment(); + $this->assertStringNotContainsString('@NoAdminRequired', $docComment); + } } From da0fe97f963ec49a9823630a1ca3bcb402a8653f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= <1005065+DeepDiver1975@users.noreply.github.com> Date: Mon, 18 May 2026 18:19:36 +0200 Subject: [PATCH 2/4] fix: replace OC.AppConfig.getValue in users.js with server-rendered checkbox state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The umgmt_set_password value is already rendered server-side into #CheckBoxPasswordOnUserCreate's checked attribute. Remove the redundant AJAX call which would now return 403 for Subadmins after the security fix. Signed-off-by: Thomas Müller <1005065+DeepDiver1975@users.noreply.github.com> --- settings/js/users/users.js | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/settings/js/users/users.js b/settings/js/users/users.js index a6d666911d1..968173332d8 100644 --- a/settings/js/users/users.js +++ b/settings/js/users/users.js @@ -751,18 +751,16 @@ $(document).ready(function () { // TODO: move other init calls inside of initialize UserList.initialize($('#userlist')); - OC.AppConfig.getValue('core', 'umgmt_set_password', 'false', function (data) { - var showPassword = $.parseJSON(data); - if (showPassword === true) { + (function () { + var showPassword = $('#CheckBoxPasswordOnUserCreate').prop('checked'); + if (showPassword) { $("#newuserpassword").show(); $("#newemail").hide(); - $('#CheckBoxPasswordOnUserCreate').attr('checked', true); } else { $("#newemail").show(); $("#newuserpassword").hide(); - $('#CheckBoxPasswordOnUserCreate').attr('checked', false); } - }); + }()); $userListBody.on('click', '.password', function (event) { event.stopPropagation(); From 91fd4669e511a6c7e962eaa0b24113340c9bf5a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= <1005065+DeepDiver1975@users.noreply.github.com> Date: Mon, 18 May 2026 23:57:16 +0200 Subject: [PATCH 3/4] docs: add entry for OC10-75 AppConfigController privilege escalation fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Sonnet 4.6 Signed-off-by: Thomas Müller <1005065+DeepDiver1975@users.noreply.github.com> --- changelog/unreleased/OC10-75 | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 changelog/unreleased/OC10-75 diff --git a/changelog/unreleased/OC10-75 b/changelog/unreleased/OC10-75 new file mode 100644 index 00000000000..048ef9d62f2 --- /dev/null +++ b/changelog/unreleased/OC10-75 @@ -0,0 +1,9 @@ +Security: Restrict AppConfigController read methods to full admins only + +Subadmin users could read all oc_appconfig values including SMTP passwords, +LDAP bind credentials, and encryption master keys via the Settings API. +Removed @NoAdminRequired from getApps, getKeys, and getValue so that the +AdminMiddleware enforces full-admin-only access, consistent with the write +methods. + +https://github.com/owncloud/core/pull/PLACEHOLDER From d00ecca39a21da3a281e00448898cff3b3034959 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BCller?= <1005065+DeepDiver1975@users.noreply.github.com> Date: Tue, 19 May 2026 00:33:07 +0200 Subject: [PATCH 4/4] docs: add PR link for OC10-75 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Sonnet 4.6 Signed-off-by: Thomas Müller <1005065+DeepDiver1975@users.noreply.github.com> --- changelog/unreleased/OC10-75 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/unreleased/OC10-75 b/changelog/unreleased/OC10-75 index 048ef9d62f2..1d4c6552653 100644 --- a/changelog/unreleased/OC10-75 +++ b/changelog/unreleased/OC10-75 @@ -6,4 +6,4 @@ Removed @NoAdminRequired from getApps, getKeys, and getValue so that the AdminMiddleware enforces full-admin-only access, consistent with the write methods. -https://github.com/owncloud/core/pull/PLACEHOLDER +https://github.com/owncloud/core/pull/41550