From 70d98ff48ac1a10e44b13121532c1458108c8ccf Mon Sep 17 00:00:00 2001 From: fds2610-bot Date: Mon, 27 Jul 2026 11:59:01 +0200 Subject: [PATCH] Fix #130: SSRF-Schutz gegen DNS-Rebinding (TOCTOU) per IP-Pinning --- Changes.md | 8 +++ src/Services/FinTsFetchService.php | 12 +++++ src/Services/NextcloudImportService.php | 16 +++++- src/Services/SsrfGuard.php | 25 ++++++++- tests/63_SsrfPinningTest.php | 71 +++++++++++++++++++++++++ 5 files changed, 129 insertions(+), 3 deletions(-) create mode 100644 tests/63_SsrfPinningTest.php diff --git a/Changes.md b/Changes.md index 586b74f..a6d25db 100644 --- a/Changes.md +++ b/Changes.md @@ -2,6 +2,14 @@ --- +## 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. +- **`src/Services/SsrfGuard.php`:** Neue Methode `resolvePinned()` gibt zusätzlich zur Validierung die geprüften IP-Adressen als `CURLOPT_RESOLVE`-Einträge (`HOST:PORT:IP`) zurück; `assertPublicHttpsUrl()` bleibt als Wrapper bestehen. +- **`src/Services/NextcloudImportService.php`:** Der Konstruktor pinnt die geprüfte(n) IP(s) der `nextcloud_url` einmalig; `newCurl()` setzt `CURLOPT_RESOLVE`, sodass alle cURL-Requests (PROPFIND/GET/MOVE/DELETE) den Hostnamen nicht erneut über den System-Resolver auflösen. +- **`src/Services/FinTsFetchService.php`:** `php-fints` löst den Hostnamen selbst auf (kein IP-Pinning möglich) – die SSRF-Prüfung wird daher zusätzlich unmittelbar vor dem ersten tatsächlichen Request (nach `FinTs::new()`) wiederholt, um das Zeitfenster für DNS-Rebinding zu minimieren. +- **`tests/63_SsrfPinningTest.php`:** Neuer Test prüft `SsrfGuard::resolvePinned()` (Einträge für Hostnamen, leeres Ergebnis + weiterhin Ablehnung für IP-Literale) sowie das Pinning in `NextcloudImportService`. + ## 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. diff --git a/src/Services/FinTsFetchService.php b/src/Services/FinTsFetchService.php index 7ac309a..a506521 100644 --- a/src/Services/FinTsFetchService.php +++ b/src/Services/FinTsFetchService.php @@ -95,6 +95,18 @@ class FinTsFetchService } $fints = $this->fints; + // Erneute Prüfung unmittelbar vor dem ersten tatsächlichen Request: php-fints + // löst den Hostnamen beim Verbindungsaufbau selbst über den System-Resolver auf + // (kein IP-Pinning wie bei den selbst gesteuerten cURL-Aufrufen in + // NextcloudImportService möglich) – die zweite Prüfung minimiert das Zeitfenster + // für DNS-Rebinding zwischen Validierung und Verbindungsaufbau (siehe Issue #130). + try { + SsrfGuard::assertPublicHttpsUrl($url); + } catch (\Throwable $e) { + $log('FEHLER: Unzulässige FinTS-URL – ' . $e->getMessage()); + return; + } + $log('TAN-Verfahren werden ermittelt…'); try { $tanModes = $fints->getTanModes(); diff --git a/src/Services/NextcloudImportService.php b/src/Services/NextcloudImportService.php index e4db152..c9542e0 100644 --- a/src/Services/NextcloudImportService.php +++ b/src/Services/NextcloudImportService.php @@ -49,6 +49,8 @@ class NextcloudImportService private string $davBase; private array $pendingEvents = []; private string|null $configError = null; + /** @var string[] CURLOPT_RESOLVE-Einträge für die beim Konstruieren geprüfte(n) IP(s) von baseUrl */ + private array $pinnedResolve = []; public function __construct() { @@ -61,10 +63,14 @@ class NextcloudImportService $this->doneFolder = $doneRaw !== '' ? ('/' . trim($doneRaw, '/')) : null; // Defensiv geprüft, auch wenn die URL beim Speichern in den Einstellungen - // bereits validiert wurde (Schutz gegen SSRF, siehe SsrfGuard). + // bereits validiert wurde (Schutz gegen SSRF, siehe SsrfGuard). Die dabei + // geprüften IP-Adressen werden für alle cURL-Requests dieser Instanz fest + // angebunden (CURLOPT_RESOLVE in newCurl()), damit cURL den Hostnamen nicht + // zu einem späteren Zeitpunkt erneut selbst auflöst (DNS-Rebinding/TOCTOU, + // siehe Issue #130). if ($this->baseUrl !== '') { try { - SsrfGuard::assertPublicHttpsUrl($this->baseUrl); + $this->pinnedResolve = SsrfGuard::resolvePinned($this->baseUrl); } catch (\RuntimeException $e) { $this->configError = 'Nextcloud-URL ungültig: ' . $e->getMessage(); $this->baseUrl = ''; @@ -401,6 +407,12 @@ class NextcloudImportService curl_setopt($ch, CURLOPT_FOLLOWLOCATION, false); curl_setopt($ch, CURLOPT_TIMEOUT, 60); curl_setopt($ch, CURLOPT_SSL_VERIFYPEER, true); + if (!empty($this->pinnedResolve)) { + // Host fest auf die beim Konstruieren geprüfte(n) IP(s) anbinden, statt + // ihn erneut über den System-Resolver aufzulösen (DNS-Rebinding-Schutz, + // siehe SsrfGuard::resolvePinned()). Host-Header/SNI bleiben unverändert. + curl_setopt($ch, CURLOPT_RESOLVE, $this->pinnedResolve); + } return $ch; } } diff --git a/src/Services/SsrfGuard.php b/src/Services/SsrfGuard.php index c836c24..0810a63 100644 --- a/src/Services/SsrfGuard.php +++ b/src/Services/SsrfGuard.php @@ -16,6 +16,23 @@ class SsrfGuard * @throws \RuntimeException wenn die URL kein zulässiges Ziel ist */ public static function assertPublicHttpsUrl(string $url): void + { + self::resolvePinned($url); + } + + /** + * Wie assertPublicHttpsUrl(), gibt aber zusätzlich die bereits geprüften + * IP-Adressen als CURLOPT_RESOLVE-Einträge ("HOST:PORT:IP") zurück. Damit + * kann der aufrufende Code den Host für den tatsächlichen Request auf genau + * diese IP(s) festnageln, statt ihn (z.B. durch cURL selbst) erneut über + * den System-Resolver aufzulösen – Schutz gegen DNS-Rebinding/TOCTOU + * zwischen Prüfung und Verbindungsaufbau (siehe Issue #130). + * + * @return string[] leer, wenn der Host bereits ein IP-Literal ist (dann + * ist kein Pinning nötig, cURL löst nichts auf) + * @throws \RuntimeException wenn die URL kein zulässiges Ziel ist + */ + public static function resolvePinned(string $url): array { $parts = parse_url($url); if ($parts === false || empty($parts['scheme']) || empty($parts['host'])) { @@ -29,10 +46,11 @@ class SsrfGuard } $host = trim((string)$parts['host'], '[]'); + $port = (int)($parts['port'] ?? 443); if (filter_var($host, FILTER_VALIDATE_IP)) { self::assertPublicIp($host); - return; + return []; } if (strtolower($host) === 'localhost') { @@ -46,6 +64,11 @@ class SsrfGuard foreach ($ips as $ip) { self::assertPublicIp($ip); } + + return array_map( + static fn(string $ip): string => sprintf('%s:%d:%s', $host, $port, $ip), + $ips + ); } private static function assertPublicIp(string $ip): void diff --git a/tests/63_SsrfPinningTest.php b/tests/63_SsrfPinningTest.php new file mode 100644 index 0000000..d8475e5 --- /dev/null +++ b/tests/63_SsrfPinningTest.php @@ -0,0 +1,71 @@ +assertNotEmpty($entries, 'resolvePinned() liefert Einträge für einen gültigen Hostnamen'); + foreach ($entries as $entry) { + $this->assert( + str_starts_with($entry, 'example.com:443:'), + 'CURLOPT_RESOLVE-Eintrag beginnt mit "example.com:443:" (Eintrag: ' . $entry . ')' + ); + } + + // IP-Literal als Host: kein Pinning nötig/möglich, resolvePinned() liefert leeres Array, + // validiert aber weiterhin gegen private/interne Bereiche. + $entries = SsrfGuard::resolvePinned('https://8.8.8.8/fints'); + $this->assertEquals([], $entries, 'resolvePinned() liefert kein Pinning für ein bereits öffentliches IP-Literal'); + + try { + SsrfGuard::resolvePinned('https://127.0.0.1/fints'); + $this->assert(false, 'resolvePinned() lehnt private IP-Literale weiterhin ab'); + } catch (\RuntimeException $e) { + $this->assert(true, 'resolvePinned() lehnt private IP-Literale weiterhin ab'); + } + + // NextcloudImportService: pinnedResolve wird beim Konstruieren aus der geprüften + // baseUrl befüllt und für alle eigenen cURL-Requests verwendet (newCurl()). + $this->configure('https://example.com', 'importuser', 'geheim'); + $svc = new NextcloudImportService(); + $pinned = $this->readProperty($svc, 'pinnedResolve'); + $this->assertNotEmpty($pinned, 'NextcloudImportService pinnt die IP(s) der geprüften baseUrl'); + $this->assert( + str_starts_with($pinned[0], 'example.com:443:'), + 'Gepinnter Eintrag bezieht sich auf den Host der baseUrl' + ); + } + + private function configure(string $url, string $user, string $pass): void + { + Database::execute( + 'UPDATE company_settings SET nextcloud_url=?, nextcloud_user=?, nextcloud_pass=? WHERE id=1', + [$url, $user, $pass] + ); + } + + private function readProperty(object $obj, string $name): mixed + { + $prop = new \ReflectionProperty($obj, $name); + $prop->setAccessible(true); + return $prop->getValue($obj); + } +}