From 254761082f7e0c7a3b07a1daf9d02c74a2bf58cc Mon Sep 17 00:00:00 2001 From: fds2610-bot Date: Mon, 27 Jul 2026 11:53:48 +0200 Subject: [PATCH] =?UTF-8?q?Fix=20#136:=20CSRF-Schutz=20f=C3=BCr=20GET=20/b?= =?UTF-8?q?ank/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 = [