diff --git a/CHANGELOG.md b/CHANGELOG.md index 02f49d0f..02bf5734 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Registry mappings of a deploy daemon (`occ app_api:daemon:registry:add`) are applied by the Kubernetes backend too; before, ExApp images were always taken from the registry in `info.xml`. A `local` mapping sets `imagePullPolicy: Never` with HaRP newer than 0.4.5, so the image must be present on every node. #1046 - Updating a deploy daemon that does not exist answers `success: false` instead of a server error. #1082 - The ExApp proxy also refuses `.` path segments and `..` segments followed by `;`, `%3F` or `%23`. #1163 +- The ExApp proxy refuses requests to the `/heartbeat`, `/init` and `/enabled` endpoints of ExApps, as HaRP does. #1168 ## [34.0.0] diff --git a/lib/Controller/ExAppProxyController.php b/lib/Controller/ExAppProxyController.php index 7f45e293..2f33d992 100644 --- a/lib/Controller/ExAppProxyController.php +++ b/lib/Controller/ExAppProxyController.php @@ -246,6 +246,14 @@ private function prepareProxy( ); return null; } + // As the ExApp may read it: cut at "?" or "#", decoded once, any case, surrounding slashes and newlines. + $target = rawurldecode(substr($other, 0, strcspn($other, '?#'))); + if (in_array(strtolower(trim($target, "/\n")), ['heartbeat', 'init', 'enabled'], true)) { + $this->logger->debug( + sprintf('Returning status 404 for "%s": only AppAPI calls this ExApp endpoint.', $other) + ); + return null; + } $exApp = $this->exAppService->getExApp($appId); if ($exApp === null) { $this->logger->debug( diff --git a/tests/php/Controller/ExAppProxyControllerTest.php b/tests/php/Controller/ExAppProxyControllerTest.php index eefef966..84e9aa3a 100644 --- a/tests/php/Controller/ExAppProxyControllerTest.php +++ b/tests/php/Controller/ExAppProxyControllerTest.php @@ -90,6 +90,31 @@ public function testAccessDeniedDoesNotFallThroughToLaterRoutes(): void { self::assertSame('^/.*', $this->routeFor('GET', 'other', $adminFirst, 'alice')['url']); } + public static function internalEndpointProvider(): array { + return [ + 'heartbeat' => ['heartbeat'], + 'init' => ['init'], + 'enabled' => ['enabled'], + 'trailing slash' => ['init/'], + 'other case' => ['Enabled'], + 'encoded leading slash' => ['%2Fheartbeat'], + 'percent-encoded' => ['%65nabled'], + 'followed by a decoded "?"' => ['enabled?enabled=0'], + 'followed by a decoded "#"' => ['init#x'], + 'followed by a newline' => ["heartbeat\n"], + ]; + } + + #[DataProvider('internalEndpointProvider')] + public function testInternalEndpointIsRefused(string $path): void { + self::assertFalse($this->isForwarded($path)); + } + + public function testPathsNearInternalEndpointsAreForwarded(): void { + self::assertTrue($this->isForwarded('heartbeats')); + self::assertTrue($this->isForwarded('api/enabled')); + } + public function testHeadIsAcceptedWhereGetIs(): void { // Nextcloud routes HEAD to the proxy's GET action, and HaRP accepts HEAD on GET routes too. self::assertSame('GET', $this->routeFor('HEAD', 'settings', self::SAME_URL)['verb']);