From 776f85bcf8e07aa029c2369d31ce01c336f82f97 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:41 +0200 Subject: [PATCH 1/2] fix(files_external): sanitize exception messages in storage status response MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The catch-all Exception handler in updateStorageStatus() set the storage statusMessage to get_class($e).": ".$e->getMessage(), which for Guzzle/cURL exceptions includes the resolved IP, port, and cURL error code. This created an internal network oracle: authenticated users with external storage access could distinguish "connection refused" from "timed out" from "no DNS" to map internal topology. Log the full exception server-side and return a generic l10n string to the client instead. Signed-off-by: Thomas Müller Signed-off-by: Thomas Müller <1005065+DeepDiver1975@users.noreply.github.com> --- .../lib/Controller/StoragesController.php | 7 +- .../Controller/StoragesControllerTest.php | 81 +++++++++++++++++++ 2 files changed, 87 insertions(+), 1 deletion(-) diff --git a/apps/files_external/lib/Controller/StoragesController.php b/apps/files_external/lib/Controller/StoragesController.php index 21de089ab642..0d2560086f1f 100644 --- a/apps/files_external/lib/Controller/StoragesController.php +++ b/apps/files_external/lib/Controller/StoragesController.php @@ -266,9 +266,14 @@ protected function updateStorageStatus(IStorageConfig &$storage, $testOnly = tru ); } catch (\Exception $e) { // FIXME: convert storage exceptions to StorageNotAvailableException + // Log the full exception server-side but do NOT expose the message to the + // client: exception messages from e.g. Guzzle contain resolved IP addresses, + // ports and cURL error details which can be used for internal network + // reconnaissance (information-disclosure / SSRF oracle). + $this->logger->logException($e, ['app' => 'files_external']); $storage->setStatus( StorageNotAvailableException::STATUS_ERROR, - \get_class($e).': '.$e->getMessage() + $this->l10n->t('Storage connection error. See server log for details.') ); } } diff --git a/apps/files_external/tests/Controller/StoragesControllerTest.php b/apps/files_external/tests/Controller/StoragesControllerTest.php index 3df57c9d04df..02841f9fbb3f 100644 --- a/apps/files_external/tests/Controller/StoragesControllerTest.php +++ b/apps/files_external/tests/Controller/StoragesControllerTest.php @@ -446,6 +446,87 @@ public function testValidateStorage($backendValidate, $authMechValidate, $expect } } + /** + * Verify that a generic \Exception thrown during updateStorageStatus does NOT leak + * the raw exception class name or message into the JSON response's statusMessage + * field (information-disclosure / SSRF-oracle fix). + * + * The vulnerability: the original catch block set statusMessage to + * get_class($e).': '.$e->getMessage() + * which exposes resolved IP addresses, ports, and cURL error details to the + * client when a Guzzle ConnectException is thrown. + */ + public function testUpdateStorageStatusDoesNotLeakExceptionDetailsInStatusMessage() { + $sensitiveMessage = 'cURL error 7: Failed to connect to 10.0.0.99 port 80: Connection refused'; + $sensitiveClass = 'GuzzleHttp\Exception\ConnectException'; + + $backend = $this->getMockBuilder('\OCP\Files\External\Backend\Backend') + ->disableOriginalConstructor() + ->getMock(); + $backend->method('getStorageClass') + ->willReturn('\OCA\Files_External\Lib\Storage\SMB'); + $backend->method('getIdentifier') + ->willReturn('identifier:\OCA\Files_External\Lib\Backend\SMB'); + // Simulate a Guzzle-style exception thrown during manipulateStorageConfig + $backend->method('manipulateStorageConfig') + ->willThrowException(new \RuntimeException($sensitiveMessage)); + + $authMech = $this->getAuthMechMock(); + + $storageConfig = new StorageConfig(1); + $storageConfig->setMountPoint('mount'); + $storageConfig->setBackend($backend); + $storageConfig->setAuthMechanism($authMech); + $storageConfig->setBackendOptions([]); + + $this->service->expects($this->once()) + ->method('getStorage') + ->with(1) + ->willReturn($storageConfig); + + // Disable skipTest so the real exception path is exercised + \OC_Mount_Config::$skipTest = false; + $response = $this->controller->show(1); + \OC_Mount_Config::$skipTest = true; + + $this->assertEquals(Http::STATUS_OK, $response->getStatus()); + + $data = $response->getData(); + $serialized = $data->jsonSerialize(); + + // The status must indicate error + $this->assertEquals( + StorageNotAvailableException::STATUS_ERROR, + $serialized['status'], + 'Storage status must be STATUS_ERROR when a generic exception is thrown' + ); + + // The statusMessage MUST NOT contain the raw exception class name or message + $this->assertArrayHasKey('statusMessage', $serialized); + $statusMessage = $serialized['statusMessage']; + + $this->assertStringNotContainsString( + $sensitiveMessage, + $statusMessage, + 'statusMessage must not expose the raw exception message (network details leak)' + ); + $this->assertStringNotContainsString( + $sensitiveClass, + $statusMessage, + 'statusMessage must not expose the exception class name' + ); + $this->assertStringNotContainsString( + '10.0.0.99', + $statusMessage, + 'statusMessage must not expose internal IP addresses' + ); + $this->assertStringNotContainsString( + 'RuntimeException', + $statusMessage, + 'statusMessage must not expose the exception class name' + ); + } + protected function getNewStorageConfigMock($params) { $backendMock = $this->getBackendMock( $params['backendClass'] ?? '\Random\Missing\Class', From 2351e65f8ac4d98ab69b65591e4034cceb747aeb 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/2] chore: add changelog entry for #41585 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/41585 | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 changelog/unreleased/41585 diff --git a/changelog/unreleased/41585 b/changelog/unreleased/41585 new file mode 100644 index 000000000000..3e0cb5d45c07 --- /dev/null +++ b/changelog/unreleased/41585 @@ -0,0 +1,10 @@ +Security: Sanitize storage connection error messages returned to clients + +The external storage status handler returned raw exception messages +including Guzzle cURL error details such as resolved IP addresses and +port numbers in the JSON response. This allowed authenticated users to +distinguish connection states and map internal network topology. The +full exception is now logged server-side only; a generic message is +returned to the client. + +https://github.com/owncloud/core/pull/41585