From 9440e1ef115ba634cf6b6993190100d54d18ad4e Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Sat, 18 Jul 2026 12:10:53 +0200 Subject: [PATCH] security: fix stored XSS in per-copy fields + harden two open redirects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Full-app XSS review. One real stored XSS plus two low-severity open redirects and three defence-in-depth encoding gaps. Stored XSS (high) — app/Views/libri/scheda_libro.php. The staff-writable per-copy fields numero_inventario/note/stato (copy tracking #238) were emitted into inline onclick handlers with only htmlspecialchars(ENT_QUOTES) — the wrong encoding for a JS string inside an HTML attribute: the browser HTML-decodes the attribute before the JS parser runs, so a copy code like ');alert(document.domain)// broke out of the string and executed in another admin's browser on click of the copy edit/delete button (a staff->admin vector, issue #27). numero_inventario was additionally dropped raw into a SweetAlert html: template. Fix: json_encode() the values (JSON_HEX_* flags) for the onclick handlers and escapeHtml() the SweetAlert body. Open redirects (low): - AutoriController::delete used str_contains(referer, '/admin/authors'), which accepts https://evil.tld/admin/authors. Now validates same-host and path before bouncing back, else falls back to url('/admin/authors'). - public/index.php force-HTTPS redirect reflected the raw Host header. Now prefers the APP_CANONICAL_URL host when configured. Defence in depth (low, not currently reachable): add JSON_HEX_TAG to the home JSON-LD (FrontendController) and to the notification 'Apri' label (layout.php); escape the bare route_path() href in the latest-books CTA. Tests: xss-copy-fields-encoding.unit.php (30 assertions — the exact view encoding is inert for every breakout payload) and xss-copy-fields.spec.js (seeds a copy with a live payload, drives the real book-detail page: no onclick breakout, no SweetAlert injection, no dialog). --- app/Controllers/AutoriController.php | 13 +- app/Controllers/FrontendController.php | 7 +- .../home-sections/latest_books_title.php | 2 +- app/Views/layout.php | 2 +- app/Views/libri/scheda_libro.php | 6 +- public/index.php | 11 ++ tests/xss-copy-fields-encoding.unit.php | 85 +++++++++++ tests/xss-copy-fields.spec.js | 132 ++++++++++++++++++ 8 files changed, 249 insertions(+), 9 deletions(-) create mode 100644 tests/xss-copy-fields-encoding.unit.php create mode 100644 tests/xss-copy-fields.spec.js diff --git a/app/Controllers/AutoriController.php b/app/Controllers/AutoriController.php index 556fa391a..8f277d514 100644 --- a/app/Controllers/AutoriController.php +++ b/app/Controllers/AutoriController.php @@ -392,8 +392,19 @@ public function delete(Request $request, Response $response, mysqli $db, int $id session_start(); } $_SESSION['error_message'] = __('Impossibile eliminare l\'autore: sono presenti libri associati.'); + // Only bounce back to the referring authors page when it is a + // same-host /admin/authors URL — a bare substring check would let + // https://evil.tld/admin/authors through as an open redirect. $referer = $request->getHeaderLine('Referer'); - $target = str_contains($referer, '/admin/authors') ? $referer : '/admin/authors'; + $target = url('/admin/authors'); + if ($referer !== '' && strpbrk($referer, "\r\n") === false && !str_starts_with($referer, '//')) { + $parsed = parse_url($referer); + $host = $parsed['host'] ?? null; + $sameHost = $host === null || $host === ($_SERVER['HTTP_HOST'] ?? ''); + if ($sameHost && str_contains((string) ($parsed['path'] ?? ''), '/admin/authors')) { + $target = $referer; + } + } return $response->withHeader('Location', $target)->withStatus(302); } diff --git a/app/Controllers/FrontendController.php b/app/Controllers/FrontendController.php index 3bd75791d..3bd876a0f 100644 --- a/app/Controllers/FrontendController.php +++ b/app/Controllers/FrontendController.php @@ -377,10 +377,11 @@ public function home(Request $request, Response $response, mysqli $db, ?Containe $orgSchema['sameAs'] = $sameAs; } - // Combine schemas - $seoSchema = json_encode([$schemaOrg, $orgSchema], JSON_UNESCAPED_SLASHES | JSON_PRETTY_PRINT); + // Combine schemas. JSON_HEX_TAG neutralises any that could + // reach the value (defence in depth on top of the strip_tags at save). + $seoSchema = json_encode([$schemaOrg, $orgSchema], JSON_UNESCAPED_SLASHES | JSON_PRETTY_PRINT | JSON_HEX_TAG); } else { - $seoSchema = json_encode($schemaOrg, JSON_UNESCAPED_SLASHES | JSON_PRETTY_PRINT); + $seoSchema = json_encode($schemaOrg, JSON_UNESCAPED_SLASHES | JSON_PRETTY_PRINT | JSON_HEX_TAG); } // Render template diff --git a/app/Views/frontend/home-sections/latest_books_title.php b/app/Views/frontend/home-sections/latest_books_title.php index 94e6f2ea9..30d5d94b5 100644 --- a/app/Views/frontend/home-sections/latest_books_title.php +++ b/app/Views/frontend/home-sections/latest_books_title.php @@ -27,7 +27,7 @@ - + diff --git a/app/Views/layout.php b/app/Views/layout.php index ad840572c..2a1c27e08 100644 --- a/app/Views/layout.php +++ b/app/Views/layout.php @@ -1318,7 +1318,7 @@ function initializeDropdowns() {

${escapeHtml(notif.message || '')}

${hasLink ? ` - +