From 64898195a30003873dd554d60f0f96d594653ff1 Mon Sep 17 00:00:00 2001
From: fds2610-bot <claude@sonnengetrock.net>
Date: Mon, 27 Jul 2026 12:07:29 +0200
Subject: [PATCH] =?UTF-8?q?Fix=20#132:=20Reflektiertes=20XSS=20=C3=BCber?=
 =?UTF-8?q?=20=5Ferror-Parameter=20behoben?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 Changes.md                               |  7 ++
 src/Controllers/InvoiceController.php    |  2 +-
 src/Views/belege/detail.php              |  2 +-
 src/Views/invoices/book.php              |  2 +-
 tests/65_InvoiceBookReflectedXssTest.php | 84 ++++++++++++++++++++++++
 5 files changed, 94 insertions(+), 3 deletions(-)
 create mode 100644 tests/65_InvoiceBookReflectedXssTest.php

diff --git a/Changes.md b/Changes.md
index 6a645c7..d042516 100644
--- a/Changes.md
+++ b/Changes.md
@@ -2,6 +2,13 @@
 
 ---
 
+## 2026-07-27 – Fix: Reflektiertes XSS über _error-Parameter bei Rechnungs-Zahlungserfassung (Issue #132)
+
+- `InvoiceController::book()` übernahm den Fehler-Array ungeprüft direkt aus `$_GET['_error']` und reichte ihn an die View weiter. Da `_error` nirgendwo serverseitig befüllt wird und der CSRF-Schutz nur für POST/PUT/DELETE/PATCH greift, konnte ein Angreifer per einfachem Link (`GET /invoices/{id}/book?_error[x]=<script>...`) beliebiges JavaScript im Kontext eines eingeloggten Nutzers ausführen und darüber auch das im `<meta>`-Tag liegende CSRF-Token auslesen.
+- **`src/Controllers/InvoiceController.php`:** `book()` übernimmt `_error` nicht mehr aus `$_GET`, `$formErrors` startet immer leer.
+- **`src/Views/invoices/book.php`, `src/Views/belege/detail.php`:** Die Fehleranzeige escaped Schlüssel und Werte jetzt konsequent mit `$h()` (zuvor ungeschütztes `implode`/`array_map`-Pattern, in `belege/detail.php` betrifft dies POST-basierte Fehler aus `BookingController`).
+- **`tests/65_InvoiceBookReflectedXssTest.php`:** Neuer Test prüft, dass ein `<script>`-Payload in `$_GET['_error']` nicht mehr unescaped in der Ausgabe von `InvoiceController::book()` erscheint.
+
 ## 2026-07-27 – Fix: CSV-Formel-Injection im Anlagelisten-Export (Issue #131)
 
 - `EuerController::anlageCsv()` schrieb `assets.bezeichnung` (frei editierbarer Text) ungefiltert per `fputcsv()` in die exportierte `Anlageliste-{year}.csv`. Eine Bezeichnung wie `=cmd|'/c calc'!A1` konnte beim Öffnen der CSV in Excel/LibreOffice zu Formelausführung führen (CSV-/Formel-Injection, CWE-1236). `BelegController`/`BookingController` waren dank eigener, privater `csvSanitize()`-Methoden bereits abgesichert.
diff --git a/src/Controllers/InvoiceController.php b/src/Controllers/InvoiceController.php
index b5187fe..a7fb793 100644
--- a/src/Controllers/InvoiceController.php
+++ b/src/Controllers/InvoiceController.php
@@ -367,7 +367,7 @@ class InvoiceController extends BaseController
 
         $bookings    = Booking::forBeleg((int)$invoice['beleg_id']);
         $accounts    = Account::all(['aktiv' => 1]);
-        $formErrors  = $_GET['_error'] ?? [];
+        $formErrors  = [];
         $formData    = [];
 
         $view = new View();
diff --git a/src/Views/belege/detail.php b/src/Views/belege/detail.php
index 5ef2d14..723f7c8 100644
--- a/src/Views/belege/detail.php
+++ b/src/Views/belege/detail.php
@@ -426,7 +426,7 @@ $suppliers = $suppliers ?? [];
                     <?php if (isset($formErrors['_exception'])): ?>
                         Fehler: <?= $h($formErrors['_exception']) ?>
                     <?php else: ?>
-                        <?= implode(' &nbsp;|&nbsp; ', array_map(fn($k,$v) => "<strong>$k:</strong> $v", array_keys($formErrors), $formErrors)) ?>
+                        <?= implode(' &nbsp;|&nbsp; ', array_map(fn($k,$v) => '<strong>'.$h($k).':</strong> '.$h($v), array_keys($formErrors), $formErrors)) ?>
                     <?php endif; ?>
                 </div>
                 <?php endif; ?>
diff --git a/src/Views/invoices/book.php b/src/Views/invoices/book.php
index eedc7b4..6dca500 100644
--- a/src/Views/invoices/book.php
+++ b/src/Views/invoices/book.php
@@ -118,7 +118,7 @@ foreach ($accounts as $acc) {
             <?php if (isset($formErrors['_exception'])): ?>
                 Fehler: <?= $h($formErrors['_exception']) ?>
             <?php else: ?>
-                <?= implode(' &nbsp;|&nbsp; ', array_map(fn($k,$v) => "<strong>$k:</strong> $v", array_keys($formErrors), $formErrors)) ?>
+                <?= implode(' &nbsp;|&nbsp; ', array_map(fn($k,$v) => '<strong>'.$h($k).':</strong> '.$h($v), array_keys($formErrors), $formErrors)) ?>
             <?php endif; ?>
         </div>
         <?php endif; ?>
diff --git a/tests/65_InvoiceBookReflectedXssTest.php b/tests/65_InvoiceBookReflectedXssTest.php
new file mode 100644
index 0000000..3fe59bc
--- /dev/null
+++ b/tests/65_InvoiceBookReflectedXssTest.php
@@ -0,0 +1,84 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * Test 65: Reflektiertes XSS über den _error-GET-Parameter bei der
+ * Zahlungserfassung (InvoiceController::book()) ist geschlossen.
+ */
+class InvoiceBookReflectedXssTest extends TestCase
+{
+    protected string $testName = 'Test 65: Kein reflektiertes XSS über _error-Parameter bei Rechnungs-Zahlungserfassung';
+
+    public function run(): void
+    {
+        $datum = date('Y-m-d');
+
+        $customerId = \Steuerkram\Models\Customer::create([
+            'nachname'     => 'XssTest',
+            'firma'        => 'XSS Test GmbH',
+            'strasse'      => 'Teststraße 1',
+            'plz'          => '10115',
+            'ort'          => 'Berlin',
+            'payment_days' => 14,
+        ]);
+
+        $invoiceId = \Steuerkram\Models\Invoice::create([
+            'customer_id'           => $customerId,
+            'betreff'               => 'Testrechnung für XSS-Test',
+            'rechnungs_datum'       => $datum,
+            'leistungszeitraum_von' => date('Y-m-01'),
+            'leistungszeitraum_bis' => date('Y-m-t'),
+            'lines'                 => [
+                [
+                    'bezeichnung' => 'Beratungsleistung',
+                    'menge'       => 1,
+                    'einheit'     => 'h',
+                    'einzelpreis' => 100.00,
+                    'ust_satz'    => 19.0,
+                ],
+            ],
+        ]);
+        $rechnungsNr = \Steuerkram\Models\Invoice::finalize($invoiceId);
+        $invoice     = \Steuerkram\Models\Invoice::find($invoiceId);
+
+        $belegNr = \Steuerkram\Models\Beleg::generateBelegNr();
+        $belegId = \Steuerkram\Models\Beleg::create([
+            'beleg_nr'      => $belegNr,
+            'filename'      => $rechnungsNr . '.pdf',
+            'storage_path'  => '',
+            'mime_type'     => 'application/pdf',
+            'file_hash'     => hash('sha256', $rechnungsNr),
+            'beleg_datum'   => $invoice['rechnungs_datum'],
+            'rechnungs_nr'  => $rechnungsNr,
+            'betrag_brutto' => $invoice['betrag_brutto'],
+            'belegtyp'      => 'Ausgangsrechnung',
+            'notes'         => 'Test Ausgangsrechnung',
+        ]);
+        \Steuerkram\Database::execute(
+            'UPDATE invoices SET beleg_id=? WHERE id=?', [$belegId, $invoiceId]
+        );
+
+        $payload      = '<script>alert(document.cookie)</script>';
+        $getBackup    = $_GET;
+        $_GET['_error'] = ['x' => $payload];
+
+        try {
+            $controller = new \Steuerkram\Controllers\InvoiceController();
+            ob_start();
+            $controller->book(['id' => (string)$invoiceId]);
+            $html = ob_get_clean();
+        } finally {
+            $_GET = $getBackup;
+        }
+
+        $this->assert(
+            !str_contains($html, $payload),
+            'Roher <script>-Payload aus $_GET[_error] erscheint nicht unescaped in der Ausgabe'
+        );
+        $this->assert(
+            !str_contains($html, '<script>alert(document.cookie)'),
+            'Kein ausführbares <script>-Tag in der Ausgabe (auch nicht durch Groß-/Kleinschreibung o.ä. verschleiert)'
+        );
+    }
+}