From 3e72f06d328002af5c876d6d0b51f20034adda9a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 4 May 2026 09:56:38 +0000 Subject: [PATCH] security: fix brute-force protection, token history leak, streaming XSS, borg password persistence, and missing cache headers Agent-Logs-Url: https://github.com/nextcloud/all-in-one/sessions/f1016d36-0771-46e0-992c-95ce22594414 Co-authored-by: szaimen <42591237+szaimen@users.noreply.github.com> --- php/public/clean-history.js | 6 ++++ php/public/index.php | 4 +-- php/src/Controller/DockerController.php | 8 +++-- php/src/Controller/LoginController.php | 40 ++++++++++++++++++++++++- 4 files changed, 53 insertions(+), 5 deletions(-) create mode 100644 php/public/clean-history.js diff --git a/php/public/clean-history.js b/php/public/clean-history.js new file mode 100644 index 00000000..731fce4a --- /dev/null +++ b/php/public/clean-history.js @@ -0,0 +1,6 @@ +// This script is loaded after a successful token-based login. +// It replaces the browser's current history entry (stripping the token from the +// URL) before navigating to the main AIO page, so the token is never left in +// the browser history and cannot be accidentally exposed via the back-button. +history.replaceState(null, '', location.pathname); +window.location.replace('../../'); diff --git a/php/public/index.php b/php/public/index.php index eb2a7878..24c93ceb 100644 --- a/php/public/index.php +++ b/php/public/index.php @@ -181,7 +181,7 @@ $app->get('/containers', function (Request $request, Response $response, array $ 'community_containers' => $configurationManager->listAvailableCommunityContainers(), 'community_containers_enabled' => $configurationManager->aioCommunityContainers, 'bypass_container_update' => $bypass_container_update, - ]); + ])->withHeader('Cache-Control', 'no-store'); })->setName('profile'); $app->get('/login', function (Request $request, Response $response, array $args) use ($container) { $view = Twig::fromRequest($request); @@ -209,7 +209,7 @@ $app->get('/setup', function (Request $request, Response $response, array $args) [ 'password' => $setup->Setup(), ] - ); + )->withHeader('Cache-Control', 'no-store'); }); $app->get('/log', function (Request $request, Response $response, array $args) use ($container) { $params = $request->getQueryParams(); diff --git a/php/src/Controller/DockerController.php b/php/src/Controller/DockerController.php index 66e8a3e2..3c0ff8f0 100644 --- a/php/src/Controller/DockerController.php +++ b/php/src/Controller/DockerController.php @@ -165,6 +165,10 @@ readonly class DockerController { $id = 'nextcloud-aio-borgbackup'; $this->PerformRecursiveContainerStart($id, true, $addToStreamingResponseBody); + // The password has been passed to the borgbackup container's environment. + // Clear it from the persistent config so it is not kept at rest longer than needed. + $this->configurationManager->borgRestorePassword = ''; + // End streaming response $this->finalizeStreamingResponse($nonbufResp); return $nonbufResp; @@ -339,7 +343,7 @@ readonly class DockerController { $body = $nonbufResp->getBody(); $addToStreamingResponseBody = function (string $message) use ($body) : void { - $body->write("
$message
"); + $body->write('
' . htmlspecialchars($message, ENT_QUOTES | ENT_HTML5) . '
'); }; $this->dockerActionManager->SystemPrune($addToStreamingResponseBody); @@ -430,7 +434,7 @@ readonly class DockerController { // if it'll actually pull an image), but which should not need to know anything about the // wanted markup or formatting. $addToStreamingResponseBody = function (Container $container, string $message) use ($nonbufResp) : void { - $nonbufResp->getBody()->write("
{$container->displayName}: {$message}
"); + $nonbufResp->getBody()->write('
' . htmlspecialchars($container->displayName, ENT_QUOTES | ENT_HTML5) . ': ' . htmlspecialchars($message, ENT_QUOTES | ENT_HTML5) . '
'); }; return $addToStreamingResponseBody; diff --git a/php/src/Controller/LoginController.php b/php/src/Controller/LoginController.php index d37a2210..c40f1553 100644 --- a/php/src/Controller/LoginController.php +++ b/php/src/Controller/LoginController.php @@ -17,17 +17,45 @@ readonly class LoginController { ) { } + /** + * Maximum number of failed login attempts allowed within the rate-limit window. + */ + private const int MAX_FAILED_ATTEMPTS = 10; + + /** + * Duration in seconds during which failed attempts are counted (and for which a lockout lasts). + */ + private const int RATE_LIMIT_WINDOW_SEC = 300; + public function TryLogin(Request $request, Response $response, array $args) : Response { if (!$this->dockerActionManager->isLoginAllowed()) { $response->getBody()->write("The login is blocked since Nextcloud is running."); return $response->withHeader('Location', '.')->withStatus(422); } + + // Per-IP rate limiting: block after MAX_FAILED_ATTEMPTS failures within RATE_LIMIT_WINDOW_SEC. + $ip = $request->getServerParams()['REMOTE_ADDR'] ?? 'unknown'; + $rateLimitKey = 'login_attempts_' . hash('sha256', $ip); + $attempts = (int)(apcu_fetch($rateLimitKey) ?: 0); + + if ($attempts >= self::MAX_FAILED_ATTEMPTS) { + // Keep a delay even when blocked so the 429 itself isn't a timing oracle. + sleep(5); + $response->getBody()->write("Too many failed login attempts. Please try again later."); + return $response->withHeader('Location', '.')->withStatus(429); + } + $password = $request->getParsedBody()['password'] ?? ''; if($this->authManager->CheckCredentials($password)) { + // Clear the counter on success. + apcu_delete($rateLimitKey); $this->authManager->SetAuthState(true); return $response->withHeader('Location', '.')->withStatus(201); } + // Increment the failed-attempts counter (expires after RATE_LIMIT_WINDOW_SEC seconds). + apcu_store($rateLimitKey, $attempts + 1, self::RATE_LIMIT_WINDOW_SEC); + // Punish failed auth attempts with a delay, as a very simple means against bots. sleep(5); @@ -39,7 +67,17 @@ readonly class LoginController { $token = $request->getQueryParams()['token'] ?? ''; if($this->authManager->CheckToken($token)) { $this->authManager->SetAuthState(true); - return $response->withHeader('Location', '../..')->withStatus(302); + // Return a minimal HTML page that uses JavaScript to replace the browser's + // current history entry (removing the token from it) before navigating to + // the main AIO page. This prevents the token from remaining in browser history. + $response->getBody()->write( + '' . + '' . + '' . + '' . + '' + ); + return $response->withHeader('Content-Type', 'text/html; charset=utf-8')->withStatus(200); } // Punish failed auth attempts with a delay, as a very simple means against bots.