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

security: use persistent HMAC key, validate clean-history target, improve comments

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:03:53 +00:00
committed by GitHub
parent 6a9e55a8de
commit ef58220c09
3 changed files with 25 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -10,6 +10,16 @@
// 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 target = document.currentScript.dataset.target;
const rawTarget = document.currentScript.dataset.target;
// Validate that the redirect target is a safe relative path (starts with '.' or '/').
// This guards against hypothetical injection (e.g. 'javascript:…') even though the
// value is server-set.
const safePattern = /^[./]/;
const unsafePattern = /^\/\//;
const target = (typeof rawTarget === 'string' && safePattern.test(rawTarget) && !unsafePattern.test(rawTarget))
? rawTarget
: '/';
history.replaceState(null, '', location.pathname);
window.location.replace(target);

5
php/src/Controller/DockerController.php
View File

@@ -167,8 +167,9 @@ readonly class DockerController {
// 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.
// ConfigurationManager.set() overwrites the stored value with an empty string and
// immediately writes the updated config to disk, so the plaintext password is removed.
// The borgRestorePassword property setter calls ConfigurationManager::set() under
// the hood (via PHP 8.4+ property hooks), which immediately overwrites the stored
// value and writes the updated config to disk.
$this->configurationManager->borgRestorePassword = '';
// End streaming response

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

@@ -34,19 +34,23 @@ readonly class LoginController {
}
// Per-IP rate limiting: block after MAX_FAILED_ATTEMPTS failures within RATE_LIMIT_WINDOW_SEC.
//
// REMOTE_ADDR is set by Caddy (the reverse proxy that sits in front of PHP-FPM inside
// the mastercontainer), which passes the real client IP. In environments where an
// additional upstream proxy forwards traffic to Caddy, operators should configure Caddy
// with `trusted_proxies` so that REMOTE_ADDR reflects the actual client.
$ip = $request->getServerParams()['REMOTE_ADDR'] ?? '';
if ($ip === '') {
// Refuse to rate-limit against a shared bucket when no IP is available.
// Refuse to fall back to a shared bucket when no IP is available.
$response->getBody()->write("Unable to determine client IP. Login refused.");
return $response->withStatus(403);
}
// Use HMAC to avoid leaking which IPs are being tracked via predictable cache-key names.
$hmacKey = (string)(apcu_fetch('login_rate_limit_hmac_key') ?: '');
if ($hmacKey === '') {
$hmacKey = bin2hex(random_bytes(16));
apcu_add('login_rate_limit_hmac_key', $hmacKey);
}
// Use HMAC so the cache keys are not predictable from IP addresses alone, preventing
// enumeration of which IPs have attempted logins via APCu key inspection.
// The HMAC key is persisted via the configuration manager so it survives cache clears
// and container restarts, preserving rate-limit state across APCu evictions.
$hmacKey = $this->dockerActionManager->GetAndGenerateSecretWrapper('RATE_LIMIT_HMAC_KEY');
$rateLimitKey = 'login_attempts_' . hash_hmac('sha256', $ip, $hmacKey);
$attempts = (int)(apcu_fetch($rateLimitKey) ?: 0);