From 1e043524fc18335123c9235a237e626ff70ae6b1 Mon Sep 17 00:00:00 2001 From: fds2610-bot Date: Wed, 12 Aug 2026 16:51:55 +0200 Subject: [PATCH] =?UTF-8?q?Fix=20#183:=20SSRF-Schutz=20f=C3=BCr=20SMTP-Ein?= =?UTF-8?q?stellungen=20(mail=5Fsmtp=5Fhost)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Changes.md | 8 +++++ src/Controllers/SettingsController.php | 12 ++++++- src/Services/SmtpMailer.php | 2 ++ src/Services/SsrfGuard.php | 33 ++++++++++++++++++ src/Views/settings/form.php | 4 ++- tests/90_SsrfGuardAssertPublicHostTest.php | 40 ++++++++++++++++++++++ 6 files changed, 97 insertions(+), 2 deletions(-) create mode 100644 tests/90_SsrfGuardAssertPublicHostTest.php diff --git a/Changes.md b/Changes.md index 7b70671..736e71e 100644 --- a/Changes.md +++ b/Changes.md @@ -2,6 +2,14 @@ --- +## 2026-08-12 – SSRF-Schutz für SMTP-Einstellungen (#183) + +- `mail_smtp_host`/`mail_smtp_port` wurden ungeprüft aus `Setting::get()` an `SmtpMailer::connect()` durchgereicht, das per `stream_socket_client()` eine TCP-/TLS-Verbindung zu genau diesem Ziel aufbaut – anders als `fints_url` (#125) und `nextcloud_url` (#127) ohne jede Prüfung auf private/interne Adressen. Über `POST /settings/mail-test` konnte ein authentifizierter Nutzer damit interne Netzwerk-/Portscans auslösen (Blind-SSRF-Oracle, CWE-918), da unterschiedliche `RuntimeException`-Nachrichten offene/geschlossene/gefilterte Ports bzw. Fremddienste erkennbar machten. +- **`src/Services/SsrfGuard.php`:** neue Methode `assertPublicHost(string $host)` – wie `assertPublicHttpsUrl()`, aber ohne vorausgesetztes `https://`-Schema (SMTP kennt kein URL-Schema). Prüft IP-Literale direkt, blockt `localhost` explizit, löst Hostnamen auf und prüft alle zurückgegebenen Adressen gegen `FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE`. +- **`src/Services/SmtpMailer.php`:** `connect()` ruft `SsrfGuard::assertPublicHost($this->host)` vor dem `stream_socket_client()`-Aufruf auf. +- **`src/Controllers/SettingsController.php`:** `update()` prüft `mail_smtp_host` defensiv bereits beim Speichern (analog zu `nextcloud_url`), damit ein ungültiger Host erst gar nicht in die Einstellungen gelangt. `mailTest()` gibt bei einem Fehler nicht mehr `$e->getMessage()` an den Client zurück, sondern eine generische Meldung; die tatsächliche Ursache landet per `error_log()` im Server-Log – entschärft die Oracle-Eigenschaft für den Fall, dass trotz Speicher-Prüfung (z.B. DNS-Rebinding zwischen Speichern und Testversand) noch unterscheidbare Fehler entstehen könnten. +- **`tests/90_SsrfGuardAssertPublicHostTest.php`** (neu): Deckt `assertPublicHost()` gegen Loopback, `localhost`, RFC1918-Adressen, die Link-Local/Metadata-IP `169.254.169.254`, IPv6-Loopback, leeren Host sowie eine akzeptierte öffentliche IP ab. + ## 2026-08-11 – Überlange Methoden zerlegt: FinTsFetchService::run(), Booking::create()/reverse(), Bank-/InvoiceController (#159) - Rein strukturelle Änderung, keine Verhaltensänderung: bestehende Codeblöcke wurden in benannte Methoden verschoben, Bedingungen, Fehlermeldungen und Ablaufreihenfolge blieben unverändert. diff --git a/src/Controllers/SettingsController.php b/src/Controllers/SettingsController.php index fc7f3ff..03c137b 100644 --- a/src/Controllers/SettingsController.php +++ b/src/Controllers/SettingsController.php @@ -50,6 +50,15 @@ class SettingsController extends BaseController } } + $smtpHost = trim($data['mail_smtp_host'] ?? ''); + if ($smtpHost !== '') { + try { + SsrfGuard::assertPublicHost($smtpHost); + } catch (\RuntimeException $e) { + $errors['mail_smtp_host'] = 'Ungültiger SMTP-Host: ' . $e->getMessage(); + } + } + // Passwortfelder: leer gelassen = bestehendes (verschlüsseltes) Passwort behalten $current = Setting::get(); foreach (['nextcloud_pass', 'mail_smtp_pass'] as $pwField) { @@ -167,7 +176,8 @@ class SettingsController extends BaseController (new EmailService())->sendTestMail($settings, $to); echo json_encode(['ok' => true]); } catch (\RuntimeException $e) { - echo json_encode(['error' => $e->getMessage()]); + error_log('[Steuerkram] SettingsController::mailTest() SMTP-Fehler: ' . $e->getMessage()); + echo json_encode(['error' => 'SMTP-Verbindung fehlgeschlagen. Bitte SMTP-Einstellungen prüfen.']); } } diff --git a/src/Services/SmtpMailer.php b/src/Services/SmtpMailer.php index 8709cab..43a9f44 100644 --- a/src/Services/SmtpMailer.php +++ b/src/Services/SmtpMailer.php @@ -56,6 +56,8 @@ class SmtpMailer private function connect(): void { + SsrfGuard::assertPublicHost($this->host); + $address = $this->encryption === 'ssl' ? 'ssl://' . $this->host : 'tcp://' . $this->host; diff --git a/src/Services/SsrfGuard.php b/src/Services/SsrfGuard.php index 18e4bda..ce330a3 100644 --- a/src/Services/SsrfGuard.php +++ b/src/Services/SsrfGuard.php @@ -83,6 +83,39 @@ class SsrfGuard return [sprintf('%s:%d:%s', $host, $port, implode(',', $addresses))]; } + /** + * Wie assertPublicHttpsUrl(), aber für Ziele ohne URL-Schema (z.B. den + * SMTP-Host aus den Mail-Einstellungen): prüft nur Host + aufgelöste + * IP-Adresse(n) gegen private/reservierte Bereiche, ohne https:// oder + * einen Pfad vorauszusetzen. + * + * @throws \RuntimeException wenn der Host kein zulässiges Ziel ist + */ + public static function assertPublicHost(string $host): void + { + $host = trim($host, " \t\n\r\0\x0B[]"); + if ($host === '') { + throw new \RuntimeException('Kein Host angegeben.'); + } + + if (filter_var($host, FILTER_VALIDATE_IP)) { + self::assertPublicIp($host); + return; + } + + if (strtolower($host) === 'localhost') { + throw new \RuntimeException('Verbindungen zu localhost sind nicht erlaubt.'); + } + + $ips = self::resolveAll($host); + if (empty($ips)) { + throw new \RuntimeException('Host konnte nicht aufgelöst werden.'); + } + foreach ($ips as $ip) { + self::assertPublicIp($ip); + } + } + private static function assertPublicIp(string $ip): void { $public = filter_var( diff --git a/src/Views/settings/form.php b/src/Views/settings/form.php index cac2f1a..e4f0c9b 100644 --- a/src/Views/settings/form.php +++ b/src/Views/settings/form.php @@ -357,7 +357,9 @@
- + +
diff --git a/tests/90_SsrfGuardAssertPublicHostTest.php b/tests/90_SsrfGuardAssertPublicHostTest.php new file mode 100644 index 0000000..3faf379 --- /dev/null +++ b/tests/90_SsrfGuardAssertPublicHostTest.php @@ -0,0 +1,40 @@ +assertRejected('127.0.0.1', 'Loopback-IP (127.0.0.1) wird abgelehnt'); + $this->assertRejected('localhost', 'localhost wird abgelehnt'); + $this->assertRejected('10.0.0.5', 'RFC1918-IP (10.0.0.5) wird abgelehnt'); + $this->assertRejected('192.168.1.1', 'RFC1918-IP (192.168.1.1) wird abgelehnt'); + $this->assertRejected('169.254.169.254', 'Link-Local/Metadata-IP (169.254.169.254) wird abgelehnt'); + $this->assertRejected('[::1]', 'IPv6-Loopback wird abgelehnt'); + $this->assertRejected('', 'Leerer Host wird abgelehnt'); + + // Öffentliche IP-Literale werden akzeptiert + try { + SsrfGuard::assertPublicHost('8.8.8.8'); + $this->assert(true, 'Öffentliches IP-Literal (8.8.8.8) wird akzeptiert'); + } catch (\RuntimeException $e) { + $this->assert(false, 'Öffentliches IP-Literal (8.8.8.8) wird akzeptiert (Fehler: ' . $e->getMessage() . ')'); + } + } + + private function assertRejected(string $host, string $message): void + { + try { + SsrfGuard::assertPublicHost($host); + $this->assert(false, $message); + } catch (\RuntimeException $e) { + $this->assert(true, $message); + } + } +}