daenamnu/nextcloud
1
0
Fork 0
mirror of https://github.com/nextcloud/all-in-one.git synced 2026-05-21 10:50:10 +00:00
Code Issues Packages Projects Releases Wiki Activity

security: null-check currentScript, handle apcu_inc failure, use apcu_fetch success param

Browse Source 
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>
This commit is contained in:
copilot-swe-agent[bot]
2026-05-04 10:09:07 +00:00
committed by GitHub
parent 79e05f33cd
commit a415c76ad2
2 changed files with 26 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
php/public/clean-history.js
View File

@@ -10,11 +10,17 @@
// We replace with location.pathname only (no query string, no hash), which
// intentionally strips the ?token=… parameter and any hash fragment from the
// recorded history entry.
const rawTarget = document.currentScript.dataset.target;
// Only accept the exact relative path we set server-side to prevent any
// potential open-redirect via a manipulated data-target value.
const target = rawTarget === '../../' ? rawTarget : '/';
// Guard against environments where document.currentScript may be null.
if (!document.currentScript) {
window.location.replace('/');
} else {
const rawTarget = document.currentScript.dataset.target;
history.replaceState(null, '', location.pathname);
window.location.replace(target);
// Only accept the exact relative path we set server-side to prevent any
// potential open-redirect via a manipulated data-target value.
const target = rawTarget === '../../' ? rawTarget : '/';
history.replaceState(null, '', location.pathname);
window.location.replace(target);
}

16
php/src/Controller/LoginController.php
View File

@@ -62,7 +62,11 @@ readonly class LoginController {
// password and other secrets, so it must be kept confidential regardless.
$hmacKey = $this->dockerActionManager->GetAndGenerateSecretWrapper('RATE_LIMIT_HMAC_KEY');
$rateLimitKey = 'login_attempts_' . hash_hmac('sha256', $ip, $hmacKey);
$attempts = (int)(apcu_fetch($rateLimitKey) ?: 0);
// Use the $success parameter to distinguish "key not found" from an APCu error.
// Since apcu_enabled() confirmed APCu is operational, false here reliably means
// the key is absent (no prior failed attempts in the current window).
$fetchedValue = apcu_fetch($rateLimitKey, $fetchSuccess);
$attempts = ($fetchSuccess === true) ? (int)$fetchedValue : 0;
if ($attempts >= self::MAX_FAILED_ATTEMPTS) {
// Return 429 immediately; the rate limit itself is sufficient protection.
@@ -82,8 +86,14 @@ readonly class LoginController {
// - apcu_add() creates the key with a TTL only on the FIRST failure in a window.
// - apcu_inc() atomically increments on subsequent failures WITHOUT resetting the TTL,
// ensuring the window always expires RATE_LIMIT_WINDOW_SEC after the first failure.
// If apcu_add() fails for a reason other than the key already existing (e.g., cache full),
// apcu_inc() will also fail and return false. In that case, fail safe by refusing the request.
if (!apcu_add($rateLimitKey, 1, self::RATE_LIMIT_WINDOW_SEC)) {
apcu_inc($rateLimitKey);
if (apcu_inc($rateLimitKey) === false) {
error_log('APCu rate limit increment failed; refusing login attempt to fail safe.');
$response->getBody()->write("Login temporarily unavailable. Please try again later.");
return $response->withStatus(503);
}
}
// Punish failed auth attempts with a delay, as a very simple means against bots.
@@ -100,6 +110,8 @@ readonly class LoginController {
// 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.
// The script is served from 'self'; same-origin scripts are already trusted under
// the 'script-src-elem self' CSP directive, so no SRI hash is needed here.
$response->getBody()->write(
'<!DOCTYPE html>' .
'<html lang="en">' .