diff --git a/config/bootstrap.php b/config/bootstrap.php index e1335af..8b4be76 100644 --- a/config/bootstrap.php +++ b/config/bootstrap.php @@ -6,6 +6,11 @@ // Define application root define('APP_ROOT', dirname(__DIR__)); +// Storage directory for uploaded files — intentionally outside the webroot +// so no uploaded content is ever directly web-accessible (items #3 & #4). +// Files are served through public/media.php which validates paths and MIME types. +define('STORAGE_ROOT', '/var/www/posterg/storage'); + // Error reporting if (php_sapi_name() === 'cli-server') { // Development mode diff --git a/docs/TODO.SECURITY.md b/docs/TODO.SECURITY.md index 3c8a3cd..1010f65 100644 --- a/docs/TODO.SECURITY.md +++ b/docs/TODO.SECURITY.md @@ -12,6 +12,10 @@ | # | Issue | Severity | Resolution | |---|-------|----------|------------| | 1 | No HTTPS — admin credentials exposed in transit | 🔴 CRITICAL | **TLS is terminated upstream by the reverse proxy** (outside nginx). nginx.conf does not need to handle TLS directly. | +| 3 | Uploaded files stored inside the webroot | 🟠 HIGH | Storage moved to `STORAGE_ROOT` (`/var/www/posterg/storage/`), defined in `config/bootstrap.php`. `formulaire.php` updated. | +| 4 | File path mismatch — media files broken and insecure | 🟠 HIGH | DB paths are now storage-relative (`theses/YEAR/ID/file`, `covers/file`). New controller `public/media.php` serves files safely. `memoire.php` and `search.php` updated to use `/media.php?path=…`. Cover recording in `formulaire.php` fixed (was never inserted into DB). | +| 5 | Rate limiter bypassed by IP spoofing (`X-Forwarded-For`) | 🟠 HIGH | `lib/RateLimit.php` `getClientIdentifier()` now uses `REMOTE_ADDR` only. | +| 6 | `.htaccess` security rules silently ignored by nginx | 🟠 HIGH | All rules ported to `nginx/posterg.conf` (CSP to `/admin/` block, `.log` denial, `autoindex off`). See `nginx/HTACCESS_TO_NGINX.md`. | | 13 | Deprecated `X-XSS-Protection` header | 🔵 LOW | **Removed** from `nginx/posterg.conf`; see `nginx/SECURITY_HEADERS.md` for rationale. | --- @@ -57,9 +61,7 @@ | # | Issue | Severity | Files | |---|-------|----------|-------| -| 6 | `.htaccess` security rules silently ignored by nginx | 🟠 HIGH | `public/admin/.htaccess` → move all headers/rules into nginx server block | -| 11 | Missing Content-Security-Policy on public pages | 🟡 MEDIUM | `nginx/posterg.conf` → add `Content-Security-Policy` header | -| 13 | Deprecated `X-XSS-Protection` header | 🔵 LOW | ✅ `nginx/posterg.conf` — header removed; documented in `nginx/SECURITY_HEADERS.md` | +| 11 | Missing Content-Security-Policy on public pages | 🟡 MEDIUM | `nginx/posterg.conf` → add `Content-Security-Policy` header to main server block (admin already has CSP via item #6) | --- @@ -72,40 +74,27 @@ --- -### File Upload / Storage +### Admin Panel — Error Logging | # | Issue | Severity | Files | |---|-------|----------|-------| -| 3 | Uploaded files stored inside the webroot | 🟠 HIGH | `public/admin/actions/formulaire.php` → move uploads to `/var/www/posterg/storage/` | -| 4 | File path mismatch — media files broken and insecure | 🟠 HIGH | `formulaire.php`, `memoire.php`, `search.php` → align stored paths with a controller-served endpoint | | 9 | `error.log` written to a web-accessible path | 🟡 MEDIUM | `public/admin/actions/formulaire.php` → use absolute path outside webroot | --- -### Rate Limiting - -| # | Issue | Severity | Files | -|---|-------|----------|-------| -| 5 | Rate limiter bypassed by IP spoofing (`X-Forwarded-For`) | 🟠 HIGH | `lib/RateLimit.php` → use `REMOTE_ADDR` only | - ---- - ### File Import | # | Issue | Severity | Files | |---|-------|----------|-------| | 12 | CSV import missing server-side MIME validation | 🟡 MEDIUM | `public/admin/import.php` → add `finfo` MIME check | - - --- ## Priority Order for Remaining Work 1. 🔴 **CRITICAL** — Item 2 (add simple PHP session auth guard) -2. 🟠 **HIGH** — Items 3, 4, 5, 6 (file storage overhaul + nginx hardening + rate-limiter fix) -3. 🟡 **MEDIUM** — Items 7, 8, 9, 11, 12 (sanitisation standardisation, session hardening, CSP, error log, CSV MIME) -4. 🔵 **LOW** — ✅ All done (items 13–16) +2. 🟡 **MEDIUM** — Items 7, 8, 9, 11, 12 (sanitisation standardisation, session hardening, CSP, error log, CSV MIME) +3. 🔵 **LOW** — ✅ All done (items 13–16) --- diff --git a/lib/RateLimit.php b/lib/RateLimit.php index 04e0a75..7e66b9f 100644 --- a/lib/RateLimit.php +++ b/lib/RateLimit.php @@ -30,17 +30,15 @@ class RateLimit { * Get client identifier (IP address) * @return string Client identifier */ - private function getClientIdentifier() { - // Try to get real IP if behind proxy - if (!empty($_SERVER['HTTP_X_FORWARDED_FOR'])) { - $ip = $_SERVER['HTTP_X_FORWARDED_FOR']; - } elseif (!empty($_SERVER['HTTP_CLIENT_IP'])) { - $ip = $_SERVER['HTTP_CLIENT_IP']; - } else { - $ip = $_SERVER['REMOTE_ADDR'] ?? 'unknown'; - } - - return $ip; + private function getClientIdentifier(): string { + // Use REMOTE_ADDR only — HTTP_X_FORWARDED_FOR and HTTP_CLIENT_IP + // are fully attacker-controlled request headers and must never be + // trusted for rate-limiting purposes (an attacker can rotate them + // freely to bypass the limiter). Nginx-level rate limiting also + // uses $binary_remote_addr for the same reason. If this app is + // ever placed behind a trusted reverse-proxy, IP extraction should + // be handled at the nginx level, not here. + return $_SERVER['REMOTE_ADDR'] ?? 'unknown'; } /** diff --git a/nginx/HTACCESS_TO_NGINX.md b/nginx/HTACCESS_TO_NGINX.md new file mode 100644 index 0000000..f9c6e6f --- /dev/null +++ b/nginx/HTACCESS_TO_NGINX.md @@ -0,0 +1,36 @@ +# `.htaccess` → nginx migration (item #6) + +> **Problem:** `public/admin/.htaccess` contained Apache-specific security +> directives that nginx **silently ignores**. None of the rules were active +> in production. + +--- + +## Rules migrated into `nginx/posterg.conf` + +| Apache `.htaccess` rule | nginx equivalent | Location | +|---|---|---| +| `Header always set X-Frame-Options "SAMEORIGIN"` | `add_header X-Frame-Options "SAMEORIGIN" always;` | main `server` block (already present) | +| `Header always set X-Content-Type-Options "nosniff"` | `add_header X-Content-Type-Options "nosniff" always;` | main `server` block (already present) | +| `Header always set X-XSS-Protection "1; mode=block"` | **Intentionally omitted** — deprecated & counterproductive; see `SECURITY_HEADERS.md` | — | +| `Header always set Referrer-Policy "strict-origin-when-cross-origin"` | `add_header Referrer-Policy "strict-origin-when-cross-origin" always;` | main `server` block (already present) | +| `Header always set Content-Security-Policy "..."` | `add_header Content-Security-Policy "..." always;` | `/admin/` location block (**added**) | +| `Options -Indexes` | `autoindex off;` | `/admin/` location block (**added**; nginx default is off, explicit for clarity) | +| ` Require all denied` | `location ~ /\.(?!well-known).*` deny | main `server` block (already present) | +| ` Require all denied` | `location ~* \.(md\|txt\|sql\|sh\|json\|gitignore)$` deny + `location ~* \.log$` deny | main `server` block (`log` rule **added**) | +| `php_flag display_errors Off` | Handled by `config/bootstrap.php` (`ini_set('display_errors', '0')`) | PHP | +| `php_flag log_errors On` | Handled by `config/bootstrap.php` (`ini_set('log_errors', '1')`) | PHP | +| `php_value error_log error.log` | Handled by `config/bootstrap.php`; should use absolute path (item #9) | PHP | + +--- + +## Status of `public/admin/.htaccess` + +The file is now **dead code** on this nginx server. It has been left in place +(harmless) so it would still work if the project were ever tested behind Apache +(e.g., `php -S` built-in server doesn't read it either). All security rules it +previously attempted to set are now enforced by nginx directly. + +--- + +*Added: 2026-02-08 — security item #6* diff --git a/nginx/SECURITY_HEADERS.md b/nginx/SECURITY_HEADERS.md index 972df3d..b499317 100644 --- a/nginx/SECURITY_HEADERS.md +++ b/nginx/SECURITY_HEADERS.md @@ -1,6 +1,6 @@ # Security Headers — nginx/posterg.conf -## Headers in use +## Headers in use (main server block — all pages) | Header | Value | Purpose | |--------|-------|---------| @@ -9,6 +9,17 @@ | `Referrer-Policy` | `strict-origin-when-cross-origin` | Limit referrer leakage | | `Permissions-Policy` | `geolocation=(), microphone=(), camera=()` | Disable unused browser APIs | +## Headers in use (`/admin/` location block) + +| Header | Value | Purpose | +|--------|-------|---------| +| `Content-Security-Policy` | `default-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; object-src 'none'; frame-ancestors 'none';` | Restrict resource origins; block embedding | +| `X-Robots-Tag` | `noindex, nofollow` | Prevent search-engine indexing of admin | + +These were previously declared in `public/admin/.htaccess` as Apache +`mod_headers` directives, which nginx silently ignores. They are now +enforced directly; see `HTACCESS_TO_NGINX.md` for the full migration log. + ## Intentionally omitted headers ### `X-XSS-Protection` @@ -21,10 +32,11 @@ This header was **removed** (was `"1; mode=block"`). to expose response bodies that would otherwise be blocked. Sending it provides no protection and may introduce risk. -**Correct mitigation:** a proper `Content-Security-Policy` header (todo item #11). +**Correct mitigation:** a proper `Content-Security-Policy` header (now done for +`/admin/`; public-page CSP is todo item #11). ## Pending headers -| Header | Status | -|--------|--------| -| `Content-Security-Policy` | ⏳ todo item #11 | +| Header | Scope | Status | +|--------|-------|--------| +| `Content-Security-Policy` | Public pages (non-admin) | ⏳ todo item #11 | diff --git a/nginx/posterg.conf b/nginx/posterg.conf index d6418ff..41ee22b 100644 --- a/nginx/posterg.conf +++ b/nginx/posterg.conf @@ -74,6 +74,13 @@ server { # Rate limiting for admin limit_req zone=admin burst=5 nodelay; + # Content-Security-Policy — ported from public/admin/.htaccess which nginx ignores (item #6). + # Tighter policy for admin: no external sources, no object embedding. + add_header Content-Security-Policy "default-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; object-src 'none'; frame-ancestors 'none';" always; + + # Disable directory listing (nginx default is off; explicit for defence-in-depth, item #6) + autoindex off; + # Allow access to public assets without authentication location ~ ^/admin/(css|js|images)/ { auth_basic off; @@ -134,6 +141,14 @@ server { try_files $uri $uri/ =404; } + # Deny access to log files — public/admin/.htaccess had this rule but Apache + # .htaccess directives are silently ignored by nginx (item #6). + location ~* \.log$ { + deny all; + access_log off; + log_not_found off; + } + # Deny access to .htaccess files (if Apache's document root concurs with nginx's) location ~ /\.ht { deny all; diff --git a/public/admin/actions/formulaire.php b/public/admin/actions/formulaire.php index 2696089..a382cb4 100644 --- a/public/admin/actions/formulaire.php +++ b/public/admin/actions/formulaire.php @@ -194,9 +194,10 @@ try { // ===== HANDLE FILE UPLOADS ===== - // Create necessary directories - $uploadBaseDir = __DIR__ . "/data/theses/{$annee}/{$identifier}/"; - $coverDir = __DIR__ . "/data/covers/"; + // Create necessary directories — outside the webroot (security items #3 & #4). + // Files are served through /media.php, never directly via a URL path. + $uploadBaseDir = STORAGE_ROOT . "/theses/{$annee}/{$identifier}/"; + $coverDir = STORAGE_ROOT . "/covers/"; if (!file_exists($uploadBaseDir)) { mkdir($uploadBaseDir, 0755, true); @@ -228,11 +229,18 @@ try { if (move_uploaded_file($couverture["tmp_name"], $targetFile)) { chmod($targetFile, 0644); - $coverPath = "data/covers/" . $safeFileName; + // Path stored relative to STORAGE_ROOT; served via /media.php?path=... + $coverPath = "covers/" . $safeFileName; - // Update thesis record with cover path - $stmt = $pdo->prepare("UPDATE theses SET identifier = ? WHERE id = ?"); - // Store cover path in remarks for now (we could add a cover_path column) + // Record cover in thesis_files (type = 'cover') + $db->insertThesisFile( + $thesisId, + 'cover', + $coverPath, + basename($couverture["name"]), + $couverture["size"], + $mimeType + ); error_log("Cover image uploaded: " . $safeFileName); } } else { @@ -284,11 +292,11 @@ try { $fileType = 'main'; } - // Insert file record + // Insert file record — path relative to STORAGE_ROOT $db->insertThesisFile( $thesisId, $fileType, - "data/theses/{$annee}/{$identifier}/" . $safeFileName, + "theses/{$annee}/{$identifier}/" . $safeFileName, basename($files["name"][$i]), $files["size"][$i], $mimeType diff --git a/public/media.php b/public/media.php new file mode 100644 index 0000000..d2fceed --- /dev/null +++ b/public/media.php @@ -0,0 +1,90 @@ +file($realFull); + +$allowedMimes = [ + 'image/jpeg', + 'image/png', + 'image/gif', + 'application/pdf', + 'video/mp4', + 'application/zip', +]; + +if (!in_array($mimeType, $allowedMimes, true)) { + http_response_code(403); + exit; +} + +// --- 4. Send response headers ------------------------------------------------- + +header('Content-Type: ' . $mimeType); +header('Content-Length: ' . filesize($realFull)); +header('X-Content-Type-Options: nosniff'); + +$ext = strtolower(pathinfo($realFull, PATHINFO_EXTENSION)); + +if (in_array($ext, ['jpg', 'jpeg', 'png', 'gif'], true)) { + // Images: cache publicly for 7 days + header('Cache-Control: public, max-age=604800'); +} elseif ($ext === 'pdf') { + // PDFs: cache for 1 day, display inline + header('Cache-Control: public, max-age=86400'); + header('Content-Disposition: inline'); +} else { + // Everything else: no public caching + header('Cache-Control: private, no-store'); +} + +// --- 5. Stream file ----------------------------------------------------------- + +readfile($realFull); diff --git a/public/memoire.php b/public/memoire.php index 5d63cb8..0ec8f3a 100644 --- a/public/memoire.php +++ b/public/memoire.php @@ -125,16 +125,16 @@ include APP_ROOT . '/includes/header.php'; ?>
- +
- <?= htmlspecialchars($file['file_name']); ?> + <?= htmlspecialchars($file['file_name']); ?>
diff --git a/public/search.php b/public/search.php index ee73591..f508886 100644 --- a/public/search.php +++ b/public/search.php @@ -341,9 +341,9 @@ include APP_ROOT . '/includes/header.php'; ?> if (!empty($item['id'])) { $files = $db->getThesisFiles($item['id']); foreach ($files as $file) { - $ext = strtolower(pathinfo($file['file_path'], PATHINFO_EXTENSION)); - if (in_array($ext, ['jpg', 'jpeg', 'png', 'gif']) && $file['file_type'] === 'main') { - $coverImage = $file['file_path']; + // Look for dedicated cover file (type = 'cover') + if ($file['file_type'] === 'cover') { + $coverImage = '/media.php?path=' . urlencode($file['file_path']); break; } }