From e70a65ffb66a1e2f8a4f1ce01f3c9dae1d3583ca Mon Sep 17 00:00:00 2001 From: Pontoporeia Date: Thu, 16 Apr 2026 12:56:06 +0200 Subject: [PATCH] fix: session boot on POST path, consolidate rate limiter via checkKey() --- TODO.md | 2 ++ public/partage/index.php | 35 ++++++++++++----------------------- src/RateLimit.php | 29 +++++++++++++++++++++++++++++ 3 files changed, 43 insertions(+), 23 deletions(-) diff --git a/TODO.md b/TODO.md index 42fd016..59c7505 100644 --- a/TODO.md +++ b/TODO.md @@ -5,3 +5,5 @@ - [x] Fix regex delimiter clash (`/` inside `[...]` broke the pattern) → switched to `#` delimiter - [x] Add PHP dev server router for /partage/ URL rewriting - [x] Add nginx location block for /partage/ pretty URLs +- [x] Fix POST path missing App::boot() (session not started before submission handler) +- [x] Fix rate limiter: was instantiating RateLimit then ignoring it, reimplementing inline; added checkKey() to RateLimit and use it diff --git a/public/partage/index.php b/public/partage/index.php index ca882fa..991d98f 100644 --- a/public/partage/index.php +++ b/public/partage/index.php @@ -29,6 +29,9 @@ if (!preg_match('#^\d{8}-[A-Z0-9+/]{8}$#', $slug)) { exit; } +// Boot for all requests: starts session, initialises DB, ensures CSRF token. +App::boot(); + // ── POST: form submission ───────────────────────────────────────────────────── if ($_SERVER['REQUEST_METHOD'] === 'POST' && $action === 'submit') { handleShareLinkSubmission($slug); @@ -36,7 +39,6 @@ if ($_SERVER['REQUEST_METHOD'] === 'POST' && $action === 'submit') { } // ── GET: render form ───────────────────────────────────────────────────────── -App::boot(); // boot database + CSRF require_once APP_ROOT . '/src/ShareLink.php'; $shareLinkModel = new ShareLink(Database::getInstance()); @@ -480,7 +482,11 @@ function renderShareLinkForm(string $slug, array $link): void */ function handleShareLinkSubmission(string $slug): void { - session_start(); + // Session already started by App::boot() on the GET path; start here + // only if somehow not yet active (e.g. direct POST without prior GET). + if (session_status() === PHP_SESSION_NONE) { + session_start(); + } require_once APP_ROOT . '/src/ShareLink.php'; require_once APP_ROOT . '/src/RateLimit.php'; @@ -496,33 +502,16 @@ function handleShareLinkSubmission(string $slug): void } // ── Rate limiting ──────────────────────────────────────────────────────── - // Allow max 5 submissions per IP per 10 minutes (per share link) + // 5 submissions per IP per 10 minutes, keyed per share link. $rateLimitCacheDir = STORAGE_ROOT . '/cache/rate_limit'; - if (!is_dir($rateLimitCacheDir)) { - @mkdir($rateLimitCacheDir, 0755, true); - } - $shareRateLimitId = 'share_' . $slug . '_' . ($_SERVER['REMOTE_ADDR'] ?? 'unknown'); - $rateLimit = new RateLimit(5, 600, $rateLimitCacheDir); + $shareRateLimitId = 'share_' . $slug . '_' . ($_SERVER['REMOTE_ADDR'] ?? 'unknown'); + $rateLimit = new RateLimit(5, 600, $rateLimitCacheDir); - // Use a custom identifier based on slug + IP so each share link is rate-limited independently - $rateLimitFile = $rateLimitCacheDir . '/' . md5($shareRateLimitId) . '.json'; - $data = []; - if (file_exists($rateLimitFile)) { - $content = file_get_contents($rateLimitFile); - $data = json_decode($content, true) ?? []; - } - $now = time(); - $data = array_filter($data, fn($ts) => ($now - $ts) < 600); - - if (count($data) >= 5) { + if (!$rateLimit->checkKey($shareRateLimitId)) { $_SESSION['_flash_error'] = 'Trop de tentatives. Veuillez réessayer plus tard.'; header('Location: /partage/' . urlencode($slug)); exit; } - $data[] = $now; - if (is_writable($rateLimitCacheDir)) { - file_put_contents($rateLimitFile, json_encode($data)); - } // ── End rate limiting ──────────────────────────────────────────────────── // Check password verification if link has a password diff --git a/src/RateLimit.php b/src/RateLimit.php index 2ca847c..33f9fd0 100644 --- a/src/RateLimit.php +++ b/src/RateLimit.php @@ -50,6 +50,35 @@ class RateLimit { return $this->cacheDir . '/' . md5($identifier) . '.json'; } + /** + * Check and record a hit for an arbitrary string key. + * Useful when the caller wants a compound key (e.g. slug + IP). + * + * @return bool True if allowed, false if rate limit exceeded + */ + public function checkKey(string $key): bool { + $file = $this->getCacheFile($key); + + $data = []; + if (file_exists($file)) { + $data = json_decode(file_get_contents($file), true) ?? []; + } + + $now = time(); + $data = array_values(array_filter($data, fn($ts) => ($now - $ts) < $this->timeWindow)); + + if (count($data) >= $this->maxRequests) { + return false; + } + + $data[] = $now; + if (is_dir($this->cacheDir) && is_writable($this->cacheDir)) { + file_put_contents($file, json_encode($data)); + } + + return true; + } + /** * Check if client has exceeded rate limit * @return bool True if allowed, false if rate limit exceeded