Fix #132: Reflektiertes XSS über _error-Parameter behoben

This commit is contained in:
fds2610-bot 2026-07-27 12:07:29 +02:00
parent fc7ad72b96
commit 64898195a3
5 changed files with 94 additions and 3 deletions

View file

@ -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.

View file

@ -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();

View file

@ -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; ?>

View file

@ -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; ?>

View file

@ -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)'
);
}
}