From fc7ad72b96e6da0c882aa71d931997e763b01848 Mon Sep 17 00:00:00 2001 From: fds2610-bot Date: Mon, 27 Jul 2026 12:03:49 +0200 Subject: [PATCH] Fix #131: CSV-Formel-Injection im Anlagelisten-Export behoben --- Changes.md | 8 +++++ src/Controllers/BaseController.php | 11 ++++++ src/Controllers/BelegController.php | 8 ----- src/Controllers/BookingController.php | 8 ----- src/Controllers/EuerController.php | 2 +- tests/64_AnlageCsvInjectionTest.php | 49 +++++++++++++++++++++++++++ 6 files changed, 69 insertions(+), 17 deletions(-) create mode 100644 tests/64_AnlageCsvInjectionTest.php diff --git a/Changes.md b/Changes.md index a6d25db..6a645c7 100644 --- a/Changes.md +++ b/Changes.md @@ -2,6 +2,14 @@ --- +## 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. +- **`src/Controllers/BaseController.php`:** Die bisher in `BelegController` und `BookingController` duplizierte `csvSanitize()`-Methode wurde als gemeinsame `protected` Methode hierher verschoben (Präfix `'` bei führendem `=`, `+`, `-` oder `@`). +- **`src/Controllers/EuerController.php`:** `anlageCsv()` sanitisiert `bezeichnung` jetzt über `$this->csvSanitize()`. +- **`src/Controllers/BelegController.php`, `src/Controllers/BookingController.php`:** Duplizierte private `csvSanitize()`-Methoden entfernt, nutzen jetzt die geerbte Methode aus `BaseController`. +- **`tests/64_AnlageCsvInjectionTest.php`:** Neuer Test prüft, dass eine Anlagenbezeichnung mit führendem `=` im CSV-Export mit vorangestelltem `'` maskiert wird. + ## 2026-07-27 – Fix: SSRF-Schutz per DNS-Rebinding umgehbar (TOCTOU) (Issue #130) - `SsrfGuard::assertPublicHttpsUrl()` löste den Hostnamen einmalig auf und prüfte die IPs, der eigentliche Request (cURL bzw. `php-fints`) löste den Hostnamen aber später erneut selbst auf. Ein Angreifer mit Kontrolle über die DNS-Antworten der konfigurierten Domain konnte zwischen Prüfung und Request von einer öffentlichen auf eine interne IP (127.0.0.1, internes Netz, Cloud-Metadata) umschalten und so die SSRF-Prüfung aus #127/#118 umgehen. diff --git a/src/Controllers/BaseController.php b/src/Controllers/BaseController.php index 7d22093..1baf425 100644 --- a/src/Controllers/BaseController.php +++ b/src/Controllers/BaseController.php @@ -13,4 +13,15 @@ class BaseController include dirname(__DIR__) . '/Views/' . $template . '.php'; return ob_get_clean(); } + + /** + * Verhindert CSV-/Formel-Injection: stellt bei führendem =, +, - oder @ ein ' voran. + */ + protected function csvSanitize(string $value): string + { + if ($value !== '' && in_array($value[0], ['=', '+', '-', '@'], true)) { + return "'" . $value; + } + return $value; + } } diff --git a/src/Controllers/BelegController.php b/src/Controllers/BelegController.php index a97d165..4a01949 100644 --- a/src/Controllers/BelegController.php +++ b/src/Controllers/BelegController.php @@ -485,12 +485,4 @@ class BelegController extends BaseController Beleg::updateHash($id, hash_file('sha256', $absPath)); echo json_encode(['ok' => true]); } - - private function csvSanitize(string $value): string - { - if ($value !== '' && in_array($value[0], ['=', '+', '-', '@'], true)) { - return "'" . $value; - } - return $value; - } } diff --git a/src/Controllers/BookingController.php b/src/Controllers/BookingController.php index 1e23129..ae84636 100644 --- a/src/Controllers/BookingController.php +++ b/src/Controllers/BookingController.php @@ -330,13 +330,5 @@ class BookingController extends BaseController View::redirect('/journal', $e->getMessage(), 'error'); } } - - private function csvSanitize(string $value): string - { - if ($value !== '' && in_array($value[0], ['=', '+', '-', '@'], true)) { - return "'" . $value; - } - return $value; - } } diff --git a/src/Controllers/EuerController.php b/src/Controllers/EuerController.php index a4a6827..0adc567 100644 --- a/src/Controllers/EuerController.php +++ b/src/Controllers/EuerController.php @@ -116,7 +116,7 @@ class EuerController extends BaseController fputcsv($out, [ $al['anlage_nr'] ?: $al['lfd_nr'], - $al['bezeichnung'], + $this->csvSanitize((string)$al['bezeichnung']), $fmtDate($al['anschaffungsdatum']), $fmtNum($al['basis']), $nd, diff --git a/tests/64_AnlageCsvInjectionTest.php b/tests/64_AnlageCsvInjectionTest.php new file mode 100644 index 0000000..8a743a9 --- /dev/null +++ b/tests/64_AnlageCsvInjectionTest.php @@ -0,0 +1,49 @@ +anlageCsv(['year' => (string)$year]); + $csv = ob_get_clean(); + + $lines = explode("\n", $csv); + $zeile = null; + foreach ($lines as $line) { + if (str_contains($line, 'TEST-CSV-INJ')) { + $zeile = $line; + break; + } + } + $this->assertNotEmpty($zeile, 'Zeile mit Test-Anlage in der CSV-Ausgabe gefunden'); + + $felder = explode(';', (string)$zeile); + $bezeichnung = $felder[1] ?? ''; + + $this->assertEquals( + "'=cmd|'/c calc'!A1", + $bezeichnung, + 'Führendes = in der Bezeichnung wird mit vorangestelltem \' entschärft' + ); + $this->assert( + !str_starts_with($bezeichnung, '='), + 'Bezeichnungsfeld beginnt nicht mehr mit = (kein von Excel interpretiertes Formelfeld)' + ); + } +}