Skip to content

Commit ed16891

Browse files
committed
fix(proxy): refuse the ExApp lifecycle endpoints like HaRP
Signed-off-by: Oleksandr Piskun <oleksandr2088@icloud.com>
1 parent acd17da commit ed16891

2 files changed

Lines changed: 33 additions & 0 deletions

File tree

‎lib/Controller/ExAppProxyController.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,14 @@ private function prepareProxy(
246246
);
247247
return null;
248248
}
249+
// As the ExApp may read it: cut at "?" or "#", decoded once, any case, surrounding slashes and newlines.
250+
$target = rawurldecode(substr($other, 0, strcspn($other, '?#')));
251+
if (in_array(strtolower(trim($target, "/\n")), ['heartbeat', 'init', 'enabled'], true)) {
252+
$this->logger->debug(
253+
sprintf('Returning status 404 for "%s": only AppAPI calls this ExApp endpoint.', $other)
254+
);
255+
return null;
256+
}
249257
$exApp = $this->exAppService->getExApp($appId);
250258
if ($exApp === null) {
251259
$this->logger->debug(

‎tests/php/Controller/ExAppProxyControllerTest.php‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,31 @@ public function testAccessDeniedDoesNotFallThroughToLaterRoutes(): void {
9090
self::assertSame('^/.*', $this->routeFor('GET', 'other', $adminFirst, 'alice')['url']);
9191
}
9292

93+
public static function internalEndpointProvider(): array {
94+
return [
95+
'heartbeat' => ['heartbeat'],
96+
'init' => ['init'],
97+
'enabled' => ['enabled'],
98+
'trailing slash' => ['init/'],
99+
'other case' => ['Enabled'],
100+
'encoded leading slash' => ['%2Fheartbeat'],
101+
'percent-encoded' => ['%65nabled'],
102+
'followed by a decoded "?"' => ['enabled?enabled=0'],
103+
'followed by a decoded "#"' => ['init#x'],
104+
'followed by a newline' => ["heartbeat\n"],
105+
];
106+
}
107+
108+
#[DataProvider('internalEndpointProvider')]
109+
public function testInternalEndpointIsRefused(string $path): void {
110+
self::assertFalse($this->isForwarded($path));
111+
}
112+
113+
public function testPathsNearInternalEndpointsAreForwarded(): void {
114+
self::assertTrue($this->isForwarded('heartbeats'));
115+
self::assertTrue($this->isForwarded('api/enabled'));
116+
}
117+
93118
public function testHeadIsAcceptedWhereGetIs(): void {
94119
// Nextcloud routes HEAD to the proxy's GET action, and HaRP accepts HEAD on GET routes too.
95120
self::assertSame('GET', $this->routeFor('HEAD', 'settings', self::SAME_URL)['verb']);

0 commit comments

Comments
 (0)