From 254761082f7e0c7a3b07a1daf9d02c74a2bf58cc Mon Sep 17 00:00:00 2001 From: fds2610-bot Date: Mon, 27 Jul 2026 11:53:48 +0200 Subject: [PATCH 1/8] =?UTF-8?q?Fix=20#136:=20CSRF-Schutz=20f=C3=BCr=20GET?= =?UTF-8?q?=20/bank/direkt/{id}/run=20erg=C3=A4nzt?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Changes.md | 8 ++++++ src/Auth.php | 13 +++++++-- src/Controllers/BankController.php | 3 +++ src/Views/bank/fints.php | 3 ++- tests/62_BankDirektRunCsrfTest.php | 43 ++++++++++++++++++++++++++++++ 5 files changed, 67 insertions(+), 3 deletions(-) create mode 100644 tests/62_BankDirektRunCsrfTest.php diff --git a/Changes.md b/Changes.md index 0fc1276..586b74f 100644 --- a/Changes.md +++ b/Changes.md @@ -2,6 +2,14 @@ --- +## 2026-07-27 – Fix: CSRF-Schutz für GET /bank/direkt/{id}/run (Issue #136) + +- Der SSE-Endpunkt `GET /bank/direkt/{id}/run` löste einen echten Bankabruf (PayPal/FinTS) allein anhand der Session aus, ohne CSRF-Prüfung – da `EventSource` nur `GET` unterstützt, griff die bestehende CSRF-Middleware (nur für `POST`/`PUT`/`DELETE`/`PATCH`) nicht. Ein Angreifer konnte per Top-Level-Navigation (SameSite=Lax) einen eingeloggten Nutzer dazu bringen, unbeabsichtigt einen Abruf für eine beliebige Verbindungs-ID auszulösen. +- **`src/Auth.php`:** `validateCsrfToken()` prüft jetzt über die neue, seiteneffektfreie `isValidCsrfToken()` (analog zu `cookieParams()`), damit sich die Vergleichslogik isoliert testen lässt. +- **`src/Controllers/BankController.php`:** `direktRun()` validiert vor dem eigentlichen Abruf das per `csrf_token`-Query-Parameter übergebene Token gegen die Session. +- **`src/Views/bank/fints.php`:** Die `EventSource`-URL hängt das bereits vorhandene, in der Seite eingebettete CSRF-Token (`meta[name="csrf-token"]`) als Query-Parameter an. +- **`tests/62_BankDirektRunCsrfTest.php`:** Neuer Test prüft `Auth::isValidCsrfToken()` gegen korrektes, falsches, leeres und fehlendes Session-Token. + ## 2026-07-27 – Feature: Netto/Brutto-Umschalter in der Buchungsmaske (Issue #137) - Beim Erfassen einer Buchung musste bisher immer der Nettobetrag eingegeben werden – lag nur der Bruttobetrag vor (z. B. von einem Kassenbon), führte die manuelle Rückrechnung regelmäßig zu Rundungsdifferenzen. diff --git a/src/Auth.php b/src/Auth.php index 047f7a8..5dc30fb 100644 --- a/src/Auth.php +++ b/src/Auth.php @@ -102,10 +102,19 @@ class Auth public static function validateCsrfToken(string $token): void { self::startSession(); - $stored = $_SESSION['csrf_token'] ?? ''; - if ($stored === '' || !hash_equals($stored, $token)) { + if (!self::isValidCsrfToken($token)) { http_response_code(403); exit('CSRF-Token ungültig.'); } } + + /** + * Reiner Vergleich ohne session_start()-Seiteneffekt, damit sich die + * Prüflogik isoliert testen lässt (siehe cookieParams()). + */ + public static function isValidCsrfToken(string $token): bool + { + $stored = $_SESSION['csrf_token'] ?? ''; + return $stored !== '' && hash_equals($stored, $token); + } } diff --git a/src/Controllers/BankController.php b/src/Controllers/BankController.php index d9b295e..dc7036a 100644 --- a/src/Controllers/BankController.php +++ b/src/Controllers/BankController.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Steuerkram\Controllers; +use Steuerkram\Auth; use Steuerkram\Config; use Steuerkram\Models\BankProvider; use Steuerkram\Models\BankSource; @@ -510,6 +511,8 @@ class BankController extends BaseController public function direktRun(array $params): void { + Auth::validateCsrfToken((string)($_GET['csrf_token'] ?? '')); + $id = (int)($params['id'] ?? 0); $source = BankSource::find($id); diff --git a/src/Views/bank/fints.php b/src/Views/bank/fints.php index 77d8572..008f0e0 100644 --- a/src/Views/bank/fints.php +++ b/src/Views/bank/fints.php @@ -470,6 +470,7 @@ $csvDateFormatOptions = [ '; + $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 + + + + XML; + file_put_contents($absPath, $xmlPayload); + + $belegId = \Steuerkram\Models\Beleg::create([ + 'beleg_nr' => $belegNr, + 'filename' => 'rechnung.xml', + 'storage_path' => $storagePath, + 'mime_type' => 'text/xml', + 'file_hash' => hash('sha256', $xmlPayload), + 'beleg_datum' => date('Y-m-d'), + 'rechnungs_nr' => null, + 'betrag_brutto' => 100.00, + 'belegtyp' => 'Eingangsrechnung', + 'notes' => 'XSLT-XSS-Test', + ]); + + $controller = new \Steuerkram\Controllers\BelegController(); + + ob_start(); + $controller->serveFile(['id' => (string)$belegId]); + ob_get_clean(); + + $headers = headers_list(); + $disposition = ''; + foreach ($headers as $header) { + if (stripos($header, 'Content-Disposition:') === 0) { + $disposition = $header; + } + } + + $this->assertNotEmpty( + $disposition, + 'Es wurde ein Content-Disposition-Header gesetzt' + ); + $this->assert( + stripos($disposition, 'attachment') !== false, + "XML-Beleg wird als attachment statt inline ausgeliefert (erhalten: {$disposition})" + ); + + @unlink($absPath); + } +} From b5c2c672c8a75bb0bb68cb1854061e489c97fc9c Mon Sep 17 00:00:00 2001 From: fds2610-bot Date: Mon, 27 Jul 2026 12:17:49 +0200 Subject: [PATCH 7/8] =?UTF-8?q?Fix=20#142:=20Produktions-H=C3=A4rtung=20de?= =?UTF-8?q?r=20PHP-Fehlerausgabe?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Changes.md | 8 +++ Dockerfile | 1 + php/error-handling.ini | 6 ++ public/index.php | 44 +++++++++++++ tests/68_ProductionErrorHardeningTest.php | 78 +++++++++++++++++++++++ 5 files changed, 137 insertions(+) create mode 100644 php/error-handling.ini create mode 100644 tests/68_ProductionErrorHardeningTest.php diff --git a/Changes.md b/Changes.md index 111093e..eb8e69f 100644 --- a/Changes.md +++ b/Changes.md @@ -2,6 +2,14 @@ --- +## 2026-07-27 – Fix: Produktions-Härtung der PHP-Fehlerausgabe (Issue #142) + +- Das Docker-Image (`php:8.3-apache`) übernahm die kompilierten PHP-Defaults, bei denen `display_errors` aktiviert ist – im offiziellen Image wird `php.ini-production` nicht automatisch aktiviert. In Kombination mit `PDO::ATTR_ERRMODE_EXCEPTION` (`src/Database.php`) und fehlendem globalem Error-Handler hätte jeder unerwartete Fehler (SQL-Fehler, DB-Verbindungsproblem, Typfehler) eine vollständige PHP-Fehlermeldung inkl. Stacktrace, absoluten Serverpfaden und ggf. SQL-Details direkt an den Client ausgegeben (CWE-209, Information Exposure Through an Error Message) – auch für nicht angemeldete Nutzer (`/login` ist vor `Auth::require()` erreichbar). +- **`php/error-handling.ini`** (neu): `display_errors = Off`, `display_startup_errors = Off`, `log_errors = On`, `error_reporting = E_ALL & ~E_DEPRECATED`. +- **`Dockerfile`:** Kopiert `php/error-handling.ini` zusätzlich zu `php/uploads.ini` nach `/usr/local/etc/php/conf.d/`. +- **`public/index.php`:** Registriert ganz zu Beginn (vor Autoload/Routing) `set_error_handler` (wandelt Warnings/Notices in `ErrorException` um), `set_exception_handler` und `register_shutdown_function` (fängt Fatal Errors, die nicht als Exception auftreten). Unerwartete Fehler werden per `error_log` serverseitig geloggt; der Client erhält nur eine generische HTTP-500-Seite ohne Details. +- **`tests/68_ProductionErrorHardeningTest.php`:** Neuer Test prüft die Ini-Direktiven, die Dockerfile-Einbindung, die Registrierung der Handler vor dem Autoload sowie dass die an den Client gesendete Fehlerseite keine Exception-Details (Message/Trace/File) enthält. + ## 2026-07-27 – Fix: Stored XSS über XSLT in hochgeladenen XML-Belegen (Issue #135) - `BelegController::serveFile()` liefert Belege mit dem in der DB hinterlegten `mime_type` und `Content-Disposition: inline` unverändert an den Browser aus. Da für Belege `application/xml`/`text/xml` als MIME-Typ erlaubt ist, konnte eine hochgeladene XML-Datei mit eingebettetem ``/`xsl:stylesheet` beim Öffnen über den „Datei öffnen“-Link (`target="_blank"`, kein `download`-Attribut) direkt im Browser (v.a. Firefox) per XSLT-Transformation zu ausführbarem `