diff --git a/Changes.md b/Changes.md index 3713275..a838a7d 100644 --- a/Changes.md +++ b/Changes.md @@ -2,6 +2,19 @@ --- +## 2026-07-16 – Fix: Stored XSS über KI-Kontierungsvorschlag (supplier_name) in Beleg-Detailansicht (Issue #129) + +- **Mittel-Hoch:** `supplier_name` aus dem per LLM erzeugten Kontierungsvorschlag (`KontierungsvorschlagService`) wird nur `trim()`t und ungeprüft in `belege.ki_vorschlag` (JSON) gespeichert. `_showKiBadgeDone()` in `src/Views/belege/detail.php` baute daraus String-Konkatenation und wies sie ohne Escaping über `innerHTML` zu. Da der Belegtext (PDF/Bild/ZUGFeRD-XML) von einem externen Rechnungssteller stammt, konnte ein präparierter Beleg das LLM per Prompt-Injection dazu bringen, für `supplier_name` HTML/JS-Payload statt eines Firmennamens auszugeben – dieser wurde beim Öffnen des Belegs im Browser-Kontext des eingeloggten Nutzers ausgeführt (CWE-79). +- **`src/Views/belege/detail.php`:** `_showKiBadgeDone()` verwendet nun die bereits vorhandene `_esc()`-Hilfsfunktion, um `v.supplier_name` vor der Einbettung in `innerHTML` HTML-zu-escapen. Andere Ausgabepfade von `supplier_name`/`buchungstext` (`.value`, `.textContent`, `.dataset`) waren bereits sicher. + +## 2026-07-12 – Fix: SSRF über ungeprüftes nextcloud_url-Feld beim Belegimport (Issue #127) + +- **Mittel:** Anders als bei `fints_url` (Issue #118) wurde die in den Einstellungen gespeicherte `nextcloud_url` weder beim Speichern noch bei der Verwendung geprüft. `NextcloudImportService` baute daraus ungefiltert die WebDAV-Basis-URL für PROPFIND-, GET- und MOVE/DELETE-Requests samt Basic-Auth-Credentials; ein authentifizierter Benutzer mit Zugriff auf die Einstellungen konnte so interne Ziele (`127.0.0.1`, internes Netz, Cloud-Metadata-Endpunkt `169.254.169.254`) über den Belegimport-Cron ansprechen (CWE-918). Zusätzlich folgte der verwendete cURL-Handle HTTP-Redirects, wodurch eine nachträgliche Host-Prüfung per Weiterleitung hätte umgangen werden können. +- **`src/Controllers/SettingsController.php`:** `update()` validiert `nextcloud_url` vor dem Speichern mit dem bereits für `fints_url` verwendeten `SsrfGuard::assertPublicHttpsUrl()`; ungültige Ziele werden mit Fehlermeldung abgelehnt statt in der Datenbank zu landen. +- **`src/Services/NextcloudImportService.php`:** Der Konstruktor prüft die geladene `nextcloud_url` defensiv erneut mit `SsrfGuard` (Schutz gegen DNS-Rebinding und gegen Altbestände, die vor diesem Fix gespeichert wurden); bei Ablehnung bleibt `baseUrl` leer und `import()` bricht sofort mit Fehlermeldung ab, ohne einen Request abzusetzen. `newCurl()` setzt `CURLOPT_FOLLOWLOCATION` nun auf `false`, damit eine Weiterleitung die Prüfung nicht umgehen kann. +- **`src/Views/settings/form.php`:** Fehleranzeige für das Nextcloud-URL-Feld ergänzt. +- **`tests/56_NextcloudSsrfTest.php`:** Neuer Test prüft, dass `http://`, private/loopback/link-local IP-Literale sowie `localhost` als `nextcloud_url` von `NextcloudImportService` abgelehnt werden (kein Request, `import()` meldet Fehler), während gültige `https://`-Ziele weiterhin funktionieren. + ## 2026-07-11 – Fix: E-Mail-Header-Injection über Kunden-Betreffvorlage `mail_subject_template` (Issue #126) - **Mittel:** Das pro Kunde konfigurierbare Feld `mail_subject_template` wurde beim Speichern nur `trim()`t, eingebettete `\r`/`\n` blieben erhalten. Beim Rechnungsversand landete der Wert ungeprüft im SMTP-`Subject:`-Header (`SmtpMailer::buildMessage()`), sodass ein authentifizierter Benutzer über das Kunden-Bearbeitungsformular per CRLF-Payload (z.B. `Rechnung %RNR%\r\nBcc: angreifer@example.com`) zusätzliche Header injizieren und den Firmen-SMTP-Account zum verdeckten Versand an beliebige Empfänger missbrauchen konnte (CWE-93, analog zu #117, dort aber nur für Empfänger-/CC-Felder behoben). diff --git a/src/Controllers/SettingsController.php b/src/Controllers/SettingsController.php index 3c287e3..d9105b5 100644 --- a/src/Controllers/SettingsController.php +++ b/src/Controllers/SettingsController.php @@ -7,6 +7,7 @@ namespace Steuerkram\Controllers; use Steuerkram\Models\BankSource; use Steuerkram\Models\Setting; use Steuerkram\Services\EmailService; +use Steuerkram\Services\SsrfGuard; use Steuerkram\View; class SettingsController extends BaseController @@ -39,6 +40,15 @@ class SettingsController extends BaseController if (empty($data['plz'])) $errors['plz'] = 'Pflichtfeld'; if (empty($data['ort'])) $errors['ort'] = 'Pflichtfeld'; + $nextcloudUrl = trim($data['nextcloud_url'] ?? ''); + if ($nextcloudUrl !== '') { + try { + SsrfGuard::assertPublicHttpsUrl($nextcloudUrl); + } catch (\RuntimeException $e) { + $errors['nextcloud_url'] = 'Ungültige Nextcloud-URL: ' . $e->getMessage(); + } + } + // Passwortfelder: leer gelassen = bestehendes (verschlüsseltes) Passwort behalten $current = Setting::get(); foreach (['nextcloud_pass', 'mail_smtp_pass'] as $pwField) { diff --git a/src/Services/NextcloudImportService.php b/src/Services/NextcloudImportService.php index 8f0d1b3..e4db152 100644 --- a/src/Services/NextcloudImportService.php +++ b/src/Services/NextcloudImportService.php @@ -48,6 +48,7 @@ class NextcloudImportService private string|null $doneFolder; private string $davBase; private array $pendingEvents = []; + private string|null $configError = null; public function __construct() { @@ -58,7 +59,19 @@ class NextcloudImportService $this->folder = '/' . trim($cfg['nextcloud_folder'] ?: 'Belege/Import', '/'); $doneRaw = $cfg['nextcloud_done'] ?? ''; $this->doneFolder = $doneRaw !== '' ? ('/' . trim($doneRaw, '/')) : null; - $this->davBase = $this->baseUrl . '/remote.php/dav/files/' . rawurlencode($this->user); + + // Defensiv geprüft, auch wenn die URL beim Speichern in den Einstellungen + // bereits validiert wurde (Schutz gegen SSRF, siehe SsrfGuard). + if ($this->baseUrl !== '') { + try { + SsrfGuard::assertPublicHttpsUrl($this->baseUrl); + } catch (\RuntimeException $e) { + $this->configError = 'Nextcloud-URL ungültig: ' . $e->getMessage(); + $this->baseUrl = ''; + } + } + + $this->davBase = $this->baseUrl . '/remote.php/dav/files/' . rawurlencode($this->user); } /** @@ -76,7 +89,8 @@ class NextcloudImportService $result = ['imported' => 0, 'skipped' => 0, 'errors' => 0, 'details' => [], 'imported_ids' => []]; if (!$this->baseUrl || !$this->user || !$this->pass) { - $msg = 'FEHLER: NEXTCLOUD_URL, NEXTCLOUD_USER und NEXTCLOUD_PASS müssen gesetzt sein.'; + $msg = $this->configError + ?? 'FEHLER: NEXTCLOUD_URL, NEXTCLOUD_USER und NEXTCLOUD_PASS müssen gesetzt sein.'; $result['details'][] = $msg; $result['errors']++; $this->persistEvent($msg); @@ -382,7 +396,9 @@ class NextcloudImportService $ch = curl_init($url); curl_setopt($ch, CURLOPT_USERPWD, $this->user . ':' . $this->pass); curl_setopt($ch, CURLOPT_RETURNTRANSFER, true); - curl_setopt($ch, CURLOPT_FOLLOWLOCATION, true); + // Redirects nicht folgen: eine Weiterleitung könnte sonst die SSRF-Prüfung + // der Nextcloud-URL (SsrfGuard) umgehen. + curl_setopt($ch, CURLOPT_FOLLOWLOCATION, false); curl_setopt($ch, CURLOPT_TIMEOUT, 60); curl_setopt($ch, CURLOPT_SSL_VERIFYPEER, true); return $ch; diff --git a/src/Views/belege/detail.php b/src/Views/belege/detail.php index ef5bb6e..b2c46a0 100644 --- a/src/Views/belege/detail.php +++ b/src/Views/belege/detail.php @@ -1050,7 +1050,7 @@ function _showKiBadgeDone(arr) { const katTxt = katOpt ? katOpt.text : v.kategorie_konto_id; const zhlTxt = zhlOpt ? zhlOpt.text : v.zahlungskonto_id; const supTxt = v.supplier_name - ? ' · Lieferant: ' + v.supplier_name + (v.supplier_found === false ? ' (neu – bitte prüfen)' : '') + ? ' · Lieferant: ' + _esc(v.supplier_name) + (v.supplier_found === false ? ' (neu – bitte prüfen)' : '') : ''; // Erste Zeile (bereits ins Formular übertragen) diff --git a/src/Views/settings/form.php b/src/Views/settings/form.php index 1f93488..61ecfc0 100644 --- a/src/Views/settings/form.php +++ b/src/Views/settings/form.php @@ -315,7 +315,9 @@ +
diff --git a/tests/56_NextcloudSsrfTest.php b/tests/56_NextcloudSsrfTest.php new file mode 100644 index 0000000..a9c98f3 --- /dev/null +++ b/tests/56_NextcloudSsrfTest.php @@ -0,0 +1,84 @@ +assertRejected('http://cloud.example.com', 'http:// wird abgelehnt'); + $this->assertRejected('https://127.0.0.1', 'Loopback-IP (127.0.0.1) wird abgelehnt'); + $this->assertRejected('https://169.254.169.254', 'Link-Local/Metadata-IP (169.254.169.254) wird abgelehnt'); + $this->assertRejected('https://10.0.0.5', 'RFC1918-IP (10.0.0.5) wird abgelehnt'); + $this->assertRejected('https://localhost', 'localhost wird abgelehnt'); + + // Gültiges Ziel: baseUrl bleibt gesetzt, kein configError, davBase korrekt gebildet. + $this->configure('https://cloud.example.com', 'importuser', 'geheim'); + $svc = new NextcloudImportService(); + $baseUrl = $this->readProperty($svc, 'baseUrl'); + $davBase = $this->readProperty($svc, 'davBase'); + $cfgError = $this->readProperty($svc, 'configError'); + + $this->assertEquals('https://cloud.example.com', $baseUrl, 'Gültige https-URL bleibt als baseUrl erhalten'); + $this->assert($cfgError === null, 'Kein configError bei gültiger URL'); + $this->assert( + str_contains((string)$davBase, '/remote.php/dav/files/importuser'), + 'davBase wird aus der gültigen baseUrl gebildet' + ); + + // Ungültiges Ziel: baseUrl wird geleert, configError gesetzt, import() bricht + // sofort ab (kein PROPFIND-Versuch gegen das interne Ziel). + $this->configure('https://127.0.0.1:9999', 'importuser', 'geheim'); + $svc = new NextcloudImportService(); + $baseUrl = $this->readProperty($svc, 'baseUrl'); + $cfgError = $this->readProperty($svc, 'configError'); + + $this->assertEquals('', $baseUrl, 'Ungültige URL: baseUrl wird geleert (fail closed)'); + $this->assert( + is_string($cfgError) && str_contains($cfgError, 'ungültig'), + 'Ungültige URL: configError beschreibt die Ablehnung' + ); + + $result = $svc->import(); + $this->assertEquals(1, $result['errors'], 'import() meldet einen Fehler statt einen Request abzusetzen'); + $this->assert( + !empty($result['details']) && str_contains($result['details'][0], 'ungültig'), + 'import()-Fehlermeldung verweist auf die ungültige Nextcloud-URL' + ); + } + + 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); + } + + private function assertRejected(string $url, string $message): void + { + $this->configure($url, 'importuser', 'geheim'); + $svc = new NextcloudImportService(); + $baseUrl = $this->readProperty($svc, 'baseUrl'); + $this->assert($baseUrl === '', $message); + } +}