Fix #149: Regressionstest für reflektiertes XSS über Kontonummer-Feld

This commit is contained in:
fds2610-bot 2026-08-09 20:25:39 +02:00
parent 9df593b824
commit c971e0acb4
2 changed files with 78 additions and 0 deletions

View file

@ -2,6 +2,11 @@
--- ---
## 2026-08-09 – Regressionstest: Reflektiertes XSS über Kontonummer-Freitextfeld (#149)
- `BookingController::store()` schreibt eine nicht auflösbare Freitext-Kontonummer (`kategorie_kontonummer`/`zahlungs_kontonummer`) ungefiltert in `$errors[...]`. Die Ausgabestelle in `src/Views/belege/detail.php:419` escaped Schlüssel und Wert bereits seit Fix #132 (Commit 6489819) konsequent mit `$h()` – der in #149 gemeldete Bug war zum Zeitpunkt der Meldung durch die dortige Änderung bereits geschlossen, nur ohne eigene Testabdeckung für diesen speziellen Eingabepfad.
- **`tests/84_BookingStoreReflectedXssTest.php`** (neu): Rendert `belege/detail.php` mit einem `<script>`-Payload in `formErrors['kategorie_konto_id']` (wie ihn `store()` bei nicht gefundener Kontonummer erzeugt) und stellt sicher, dass der Payload nur escaped in der Ausgabe erscheint.
## 2026-08-09 – Fix: Fehlende Autorisierung in der Benutzerverwaltung (#147) ## 2026-08-09 – Fix: Fehlende Autorisierung in der Benutzerverwaltung (#147)
- `users` hatte kein Rollen-Feld – `Auth::require()` prüfte für alle `/useradmin`-Routen (`GET/POST /useradmin`, `.../{id}`, `.../{id}/delete`, `.../{id}/toggle`) nur eine gültige Session, keine besondere Berechtigung. Jeder eingeloggte Benutzer (auch einer, der nur für die Belegerfassung angelegt wurde) konnte damit beliebige Konten anlegen, das Passwort jedes anderen Benutzers – inklusive des ursprünglichen Admin-Accounts – setzen sowie andere Benutzer löschen oder deaktivieren (Privilege Escalation, CWE-862/CWE-269). - `users` hatte kein Rollen-Feld – `Auth::require()` prüfte für alle `/useradmin`-Routen (`GET/POST /useradmin`, `.../{id}`, `.../{id}/delete`, `.../{id}/toggle`) nur eine gültige Session, keine besondere Berechtigung. Jeder eingeloggte Benutzer (auch einer, der nur für die Belegerfassung angelegt wurde) konnte damit beliebige Konten anlegen, das Passwort jedes anderen Benutzers – inklusive des ursprünglichen Admin-Accounts – setzen sowie andere Benutzer löschen oder deaktivieren (Privilege Escalation, CWE-862/CWE-269).

View file

@ -0,0 +1,73 @@
<?php
declare(strict_types=1);
/**
* Test 84: Reflektiertes XSS über kategorie_kontonummer/zahlungs_kontonummer
* bei der Buchungserfassung (BookingController::store()) ist geschlossen.
*/
class BookingStoreReflectedXssTest extends TestCase
{
protected string $testName = 'Test 84: Kein reflektiertes XSS über Kontonummer-Freitextfeld bei Buchungserfassung';
public function run(): void
{
$datum = date('Y-m-d');
$belegNr = 'XSS-KTO-' . uniqid();
$belegId = \Steuerkram\Models\Beleg::create([
'beleg_nr' => $belegNr,
'filename' => 'xss-test.pdf',
'storage_path' => '',
'mime_type' => 'application/pdf',
'file_hash' => hash('sha256', $belegNr),
'beleg_datum' => $datum,
'belegtyp' => 'Eingangsrechnung',
]);
$this->assertGreaterThan(0, $belegId, 'Beleg angelegt');
$payload = '<script>alert(document.cookie)</script>';
// Nachbau der Fehlerauflösung aus BookingController::store() (Zeile 100-105):
// eine nicht auflösbare Freitext-Kontonummer landet unescaped in $errors.
$errors = [
'kategorie_konto_id' => "Kontonummer '$payload' nicht gefunden.",
];
// BookingController::renderBelegDetail() nur bis zum Content-Fragment
// nachgebaut (belege/detail.php), ohne layout/base.php – ein zweiter
// voller Seiten-Render im selben Testlauf würde an der Funktions-
// deklaration in layout/nav.php scheitern (siehe Test 65).
$beleg = \Steuerkram\Models\Beleg::find($belegId);
$accounts = \Steuerkram\Models\Account::all(['aktiv' => 1]);
$controller = new \Steuerkram\Controllers\BookingController();
$renderView = new \ReflectionMethod($controller, 'renderView');
$renderView->setAccessible(true);
$html = $renderView->invoke($controller, 'belege/detail', [
'beleg' => $beleg,
'bookings' => \Steuerkram\Models\Booking::forBeleg($belegId),
'integrity' => \Steuerkram\Models\Beleg::verifyIntegrity($belegId),
'accounts' => $accounts,
'assets' => \Steuerkram\Models\Asset::all(),
'formErrors' => $errors,
'formData' => [],
'bankTx' => null,
'bankTxBrutto' => 0.0,
'linkedBankTx' => null,
]);
$this->assert(
!str_contains($html, $payload),
'Roher <script>-Payload aus kategorie_kontonummer 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)'
);
$this->assert(
str_contains($html, htmlspecialchars($payload, ENT_QUOTES, 'UTF-8')),
'Escapte Version der Kontonummer erscheint in der Fehlermeldung (Kontonummer wurde nicht gefunden)'
);
}
}