Fix #133: DOM-XSS in der Bankimport-Vorschau behoben
This commit is contained in:
parent
64898195a3
commit
8000225a05
3 changed files with 70 additions and 1 deletions
|
|
@ -2,6 +2,12 @@
|
|||
|
||||
---
|
||||
|
||||
## 2026-07-27 – Fix: DOM-XSS in der CSV/XLSX-Vorschau beim Bankimport (Issue #133)
|
||||
|
||||
- `BankController::detectFormat()` liefert die aus einer hochgeladenen Bank-Umsatzdatei geparste `header_row`/`first_data` unverändert als JSON zurück; `src/Views/bank/upload.php` rendert diese Werte per `innerHTML` in ein `title="..."`-Attribut. Die dortige `esc()`-Funktion escapte bisher nur `&`, `<` und `>`, nicht aber `"`. Ein Header-/Zellwert wie `Spalte" onmouseover="alert(document.cookie)` konnte damit aus dem Attribut ausbrechen und beliebigen JavaScript-Code im Kontext des eingeloggten Nutzers ausführen (CWE-79), z.B. um das im `<meta>`-Tag liegende CSRF-Token auszulesen. Bank-Exportdateien stammen typischerweise von Dritten (Bank, per E-Mail zugeschickter Kontoauszug) und sind damit ein realistischer, angreiferkontrollierter Eingabekanal.
|
||||
- **`src/Views/bank/upload.php`:** `esc()` escaped jetzt zusätzlich `"` (zu `"`) und `'` (zu `'`).
|
||||
- **`tests/66_BankUploadPreviewXssTest.php`:** Neuer Test prüft per Quelltext-Check, dass `esc()` Anführungszeichen escaped, und führt die extrahierte JS-Funktion (falls `node` verfügbar ist) mit einem bösartigen Zellwert aus, um zu verifizieren, dass kein Ausbruch aus dem `title`-Attribut mehr möglich ist.
|
||||
|
||||
## 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.
|
||||
|
|
|
|||
|
|
@ -131,7 +131,9 @@
|
|||
}
|
||||
|
||||
function esc(str) {
|
||||
return String(str ?? '').replace(/&/g,'&').replace(/</g,'<').replace(/>/g,'>');
|
||||
return String(str ?? '')
|
||||
.replace(/&/g,'&').replace(/</g,'<').replace(/>/g,'>')
|
||||
.replace(/"/g,'"').replace(/'/g,''');
|
||||
}
|
||||
|
||||
function renderHeaderRow(headerRow, firstData) {
|
||||
|
|
|
|||
61
tests/66_BankUploadPreviewXssTest.php
Normal file
61
tests/66_BankUploadPreviewXssTest.php
Normal file
|
|
@ -0,0 +1,61 @@
|
|||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
/**
|
||||
* Test 66: Attribut-basiertes DOM-XSS in der CSV/XLSX-Vorschau beim
|
||||
* Bankimport (bank/upload.php) ist geschlossen.
|
||||
*
|
||||
* Die clientseitige esc()-Funktion escapte bisher nur &, < und >, nicht
|
||||
* aber " und '. Da escapte Werte per innerHTML in ein title="..."-Attribut
|
||||
* eingesetzt werden, konnte ein Header-/Zellwert wie
|
||||
* `Spalte" onmouseover="alert(1)` aus dem Attribut ausbrechen.
|
||||
*/
|
||||
class BankUploadPreviewXssTest extends TestCase
|
||||
{
|
||||
protected string $testName = 'Test 66: Kein DOM-XSS in der Bankimport-Vorschau (esc() escaped Anführungszeichen)';
|
||||
|
||||
public function run(): void
|
||||
{
|
||||
$viewFile = __DIR__ . '/../src/Views/bank/upload.php';
|
||||
$content = file_get_contents($viewFile);
|
||||
|
||||
$this->assert(
|
||||
$content !== false,
|
||||
'View-Datei bank/upload.php ist lesbar'
|
||||
);
|
||||
|
||||
$this->assert(
|
||||
str_contains($content, "replace(/\"/g,'"')")
|
||||
&& str_contains($content, "replace(/'/g,''')"),
|
||||
'esc() escaped sowohl " als auch \' zusätzlich zu &, < und >'
|
||||
);
|
||||
|
||||
$node = trim((string)shell_exec('command -v node'));
|
||||
if ($node === '') {
|
||||
$this->assert(true, 'node nicht verfügbar – Escaping nur per Quelltext-Check geprüft');
|
||||
return;
|
||||
}
|
||||
|
||||
if (!preg_match('/function esc\(str\) \{.*?\n \}/s', $content, $m)) {
|
||||
$this->assert(false, 'esc()-Funktion konnte aus der View-Datei extrahiert werden');
|
||||
return;
|
||||
}
|
||||
|
||||
$script = $m[0] . "\nconsole.log(esc('Spalte\" onmouseover=\"alert(1)'));";
|
||||
$tmp = tempnam(sys_get_temp_dir(), 'esc_test_') . '.js';
|
||||
file_put_contents($tmp, $script);
|
||||
$output = shell_exec(escapeshellarg($node) . ' ' . escapeshellarg($tmp) . ' 2>&1');
|
||||
@unlink($tmp);
|
||||
|
||||
$this->assert(
|
||||
$output !== null && !str_contains($output, '" onmouseover="'),
|
||||
'esc() liefert für einen bösartigen Zellwert kein aus dem title-Attribut ausbrechendes Ergebnis'
|
||||
);
|
||||
|
||||
$this->assert(
|
||||
$output !== null && str_contains($output, '"'),
|
||||
'esc() liefert für einen bösartigen Zellwert das erwartete "-Escaping'
|
||||
);
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue