From 43200d7dfe1d4acc4c37ad29771bdaa89b03da82 Mon Sep 17 00:00:00 2001 From: Geoff Moore <84508186+GeoffMoore1979@users.noreply.github.com> Date: Fri, 2 Oct 2026 11:24:00 +0100 Subject: [PATCH] fix: suppress verbose PHP errors and bound login/audit housekeeping cost - bootstrap.php: disable display_errors/display_startup_errors in favour of PHP's own error log, so an uncaught engine error can never leak a stack trace or filesystem path to a client. - LoginRateLimiter: cap tracked address/identity buckets at 20000, evicting the least recently active first, so a distributed flood of distinct addresses or login names cannot grow login-attempts.json without bound. - Logger: prune expired audit logs probabilistically (1-in-100) instead of on every request, since every request already builds a Logger and a full directory scan on each one scales with total rotated log files. Found during a self-review security audit; full findings available on request. Co-Authored-By: Claude Sonnet 5 --- cms/bootstrap.php | 6 ++++++ cms/src/NeoCMS/Logger.php | 4 +++- cms/src/NeoCMS/LoginRateLimiter.php | 13 +++++++++++++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/cms/bootstrap.php b/cms/bootstrap.php index eeaa439..7c63560 100644 --- a/cms/bootstrap.php +++ b/cms/bootstrap.php @@ -1,6 +1,12 @@ logsDir, 0700); - if ($this->auditEnabled) { + // Pruning scans every rotated log file; a Logger is built on every request, so do this + // occasionally rather than on each one (the same probabilistic approach as PHP's own session GC). + if ($this->auditEnabled && random_int(1, 100) === 1) { $this->pruneExpiredLogs(); } } diff --git a/cms/src/NeoCMS/LoginRateLimiter.php b/cms/src/NeoCMS/LoginRateLimiter.php index 70fbcb3..e616f64 100644 --- a/cms/src/NeoCMS/LoginRateLimiter.php +++ b/cms/src/NeoCMS/LoginRateLimiter.php @@ -16,6 +16,9 @@ final class LoginRateLimiter private int $maxIdentityAttempts; private int $maxAddressAttempts; + /** Hard ceiling on tracked buckets, so a distributed flood of distinct addresses or login names cannot grow the state file without bound. */ + private const MAX_TRACKED_KEYS = 20000; + /** Configure the protected state file and throttling thresholds. */ public function __construct(string $dataDirectory, array $options = []) { @@ -93,6 +96,16 @@ final class LoginRateLimiter } } unset($record); + + if (count($state) > self::MAX_TRACKED_KEYS) { + // Evict the least recently active buckets first; an evicted bucket simply restarts its count. + uasort($state, static function (array $a, array $b): int { + $lastA = max((int) ($a['locked_until'] ?? 0), $a['attempts'] ? (int) end($a['attempts']) : 0); + $lastB = max((int) ($b['locked_until'] ?? 0), $b['attempts'] ? (int) end($b['attempts']) : 0); + return $lastA <=> $lastB; + }); + $state = array_slice($state, -self::MAX_TRACKED_KEYS, null, true); + } } /** Serialise state updates beneath an exclusive lock to avoid concurrent lost updates. */