Fix security issues from audit: gate partage fragments on share_active session, add CSRF to retry-email POST, remove dead App::verifyCsrf()

This commit is contained in:
Pontoporeia
2026-06-24 14:17:08 +02:00
parent 84869ad968
commit 0062b29678
4 changed files with 42 additions and 62 deletions

View File

@@ -16,9 +16,9 @@
- [x] #build-cssfix Fix stray `}` syntax error in admin.css line 305 ✓
## Pending
- [ ] #sec-fragments-auth Gate partagé fragments on share_active session + CSRF `(partage/fragments/*.php, partage/index.php)`
- [ ] #sec-retry-csrf Add CSRF check to partage/retry-email.php POST
- [ ] #sec-cleanup-dead-code Remove dead App::verifyCsrf() or refactor action handlers to use it
- [x] #sec-fragments-auth Gate partagé fragments on share_active session (read-only fragment renderers — no CSRF needed) ✓
- [x] #sec-retry-csrf Add CSRF check to partage/retry-email.php POST
- [x] #sec-cleanup-dead-code Remove dead App::verifyCsrf() or refactor action handlers to use it
- [ ] #rep-student-touch Replace hover student popover with tap-to-open drawer for mobile `(repertoire.php, repertoire.css)`
- [ ] #rep-polish Polish: scroll-position memory on HTMX swap, animation tuning `(repertoire.css)`
- [ ] #icon-color-verify Verify icon colors render correctly across all pages (header, admin tables, forms, dialogs, cleanup modal)

View File

@@ -38,6 +38,15 @@ if ($slug === 'actions') {
// Special route: /partage/fragments/* (HTMX fragments under fragments/ subdirectory)
if ($slug === 'fragments' && $_SERVER['REQUEST_METHOD'] === 'POST') {
App::boot();
// Auth gate: fragments must be requested from an active share-link session.
// These are read-only renderers (no side effects), so CSRF is not required.
if (empty($_SESSION['share_active'])) {
http_response_code(403);
header('Content-Type: text/plain');
die('Accès refusé.');
}
$fragmentBase = $action;
$fragmentFile = __DIR__ . '/fragments/' . $fragmentBase;
if ($fragmentBase !== '' && file_exists($fragmentFile)) {
@@ -49,52 +58,26 @@ if ($slug === 'fragments' && $_SERVER['REQUEST_METHOD'] === 'POST') {
exit;
}
// Legacy routes — kept for backward compatibility, delegate to fragments/
if ($slug === 'licence-fragment' && $_SERVER['REQUEST_METHOD'] === 'POST') {
// Legacy routes — kept for backward compatibility, delegate to fragments/.
// All routed through the shared fragment dispatcher for auth/CSRF gating.
$legacyFragmentMap = [
'licence-fragment' => 'licence',
'language-autre-fragment' => 'language-autre',
'language-search-fragment' => 'language-search',
'tag-search-fragment' => 'tag-search',
'format-website-fragment' => 'format-website',
'fichiers-fragment' => 'fichiers',
'validate-file-fragment' => 'validate-file',
'pill-search-fragment' => 'pill-search',
];
if (isset($legacyFragmentMap[$slug]) && $_SERVER['REQUEST_METHOD'] === 'POST') {
App::boot();
require_once __DIR__ . '/fragments/licence.php';
exit;
if (empty($_SESSION['share_active'])) {
http_response_code(403);
header('Content-Type: text/plain');
die('Accès refusé.');
}
if ($slug === 'language-autre-fragment' && $_SERVER['REQUEST_METHOD'] === 'POST') {
App::boot();
require_once __DIR__ . '/fragments/language-autre.php';
exit;
}
if ($slug === 'language-search-fragment' && $_SERVER['REQUEST_METHOD'] === 'POST') {
App::boot();
require_once __DIR__ . '/fragments/language-search.php';
exit;
}
if ($slug === 'tag-search-fragment' && $_SERVER['REQUEST_METHOD'] === 'POST') {
App::boot();
require_once __DIR__ . '/fragments/tag-search.php';
exit;
}
if ($slug === 'format-website-fragment' && $_SERVER['REQUEST_METHOD'] === 'POST') {
App::boot();
require_once __DIR__ . '/fragments/format-website.php';
exit;
}
if ($slug === 'fichiers-fragment' && $_SERVER['REQUEST_METHOD'] === 'POST') {
App::boot();
require_once __DIR__ . '/fragments/fichiers.php';
exit;
}
if ($slug === 'validate-file-fragment' && $_SERVER['REQUEST_METHOD'] === 'POST') {
App::boot();
require_once __DIR__ . '/fragments/validate-file.php';
exit;
}
if ($slug === 'pill-search-fragment' && $_SERVER['REQUEST_METHOD'] === 'POST') {
App::boot();
require_once __DIR__ . '/fragments/pill-search.php';
require_once __DIR__ . '/fragments/' . $legacyFragmentMap[$slug] . '.php';
exit;
}

View File

@@ -26,6 +26,16 @@ $smtpError = $_SESSION['share_email_retry_error'] ?? '';
// ── POST: retry send ──────────────────────────────────────────────────────────
if ($_SERVER['REQUEST_METHOD'] === 'POST') {
// CSRF check
if (
!isset($_POST['csrf_token'], $_SESSION['csrf_token'])
|| !hash_equals($_SESSION['csrf_token'], $_POST['csrf_token'])
) {
http_response_code(403);
echo 'Token de sécurité invalide.';
exit;
}
// Skip button
if (isset($_POST['skip'])) {
unset($_SESSION['share_email_retry_thesis'], $_SESSION['share_email_retry_error']);
@@ -98,6 +108,7 @@ $pageTitle = 'Corriger l\'adresse e-mail';
<p>Votre TFE est enregistré — vous pouvez corriger votre adresse ci-dessous pour recevoir le récapitulatif, ou continuer sans e-mail.</p>
<form method="post" action="/partage/retry-email?id=<?= urlencode((string)$thesisId) ?>" class="retry-email-form">
<input type="hidden" name="csrf_token" value="<?= htmlspecialchars($_SESSION['csrf_token']) ?>">
<div class="field-wrap">
<label for="confirmation_email">Adresse e-mail corrigée</label>
<input

View File

@@ -61,20 +61,6 @@ class App
}
}
/**
* Validate the CSRF token on a POST request.
* Halts with 403 if the token is missing or invalid.
*/
public static function verifyCsrf(): void
{
if ($_SERVER['REQUEST_METHOD'] !== 'POST'
|| !isset($_POST['csrf_token'], $_SESSION['csrf_token'])
|| !hash_equals($_SESSION['csrf_token'], $_POST['csrf_token'])) {
http_response_code(403);
exit('CSRF token invalide.');
}
}
/**
* Regenerate the CSRF token after a successful mutation.
*/