Fix #127: SSRF-Schutz für nextcloud_url beim Belegimport
This commit is contained in:
parent
c006b1d00a
commit
e3c47f7d89
5 changed files with 123 additions and 3 deletions
|
|
@ -2,6 +2,14 @@
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
## 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)
|
## 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).
|
- **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).
|
||||||
|
|
|
||||||
|
|
@ -7,6 +7,7 @@ namespace Steuerkram\Controllers;
|
||||||
use Steuerkram\Models\BankSource;
|
use Steuerkram\Models\BankSource;
|
||||||
use Steuerkram\Models\Setting;
|
use Steuerkram\Models\Setting;
|
||||||
use Steuerkram\Services\EmailService;
|
use Steuerkram\Services\EmailService;
|
||||||
|
use Steuerkram\Services\SsrfGuard;
|
||||||
use Steuerkram\View;
|
use Steuerkram\View;
|
||||||
|
|
||||||
class SettingsController extends BaseController
|
class SettingsController extends BaseController
|
||||||
|
|
@ -39,6 +40,15 @@ class SettingsController extends BaseController
|
||||||
if (empty($data['plz'])) $errors['plz'] = 'Pflichtfeld';
|
if (empty($data['plz'])) $errors['plz'] = 'Pflichtfeld';
|
||||||
if (empty($data['ort'])) $errors['ort'] = '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
|
// Passwortfelder: leer gelassen = bestehendes (verschlüsseltes) Passwort behalten
|
||||||
$current = Setting::get();
|
$current = Setting::get();
|
||||||
foreach (['nextcloud_pass', 'mail_smtp_pass'] as $pwField) {
|
foreach (['nextcloud_pass', 'mail_smtp_pass'] as $pwField) {
|
||||||
|
|
|
||||||
|
|
@ -48,6 +48,7 @@ class NextcloudImportService
|
||||||
private string|null $doneFolder;
|
private string|null $doneFolder;
|
||||||
private string $davBase;
|
private string $davBase;
|
||||||
private array $pendingEvents = [];
|
private array $pendingEvents = [];
|
||||||
|
private string|null $configError = null;
|
||||||
|
|
||||||
public function __construct()
|
public function __construct()
|
||||||
{
|
{
|
||||||
|
|
@ -58,7 +59,19 @@ class NextcloudImportService
|
||||||
$this->folder = '/' . trim($cfg['nextcloud_folder'] ?: 'Belege/Import', '/');
|
$this->folder = '/' . trim($cfg['nextcloud_folder'] ?: 'Belege/Import', '/');
|
||||||
$doneRaw = $cfg['nextcloud_done'] ?? '';
|
$doneRaw = $cfg['nextcloud_done'] ?? '';
|
||||||
$this->doneFolder = $doneRaw !== '' ? ('/' . trim($doneRaw, '/')) : null;
|
$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' => []];
|
$result = ['imported' => 0, 'skipped' => 0, 'errors' => 0, 'details' => [], 'imported_ids' => []];
|
||||||
|
|
||||||
if (!$this->baseUrl || !$this->user || !$this->pass) {
|
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['details'][] = $msg;
|
||||||
$result['errors']++;
|
$result['errors']++;
|
||||||
$this->persistEvent($msg);
|
$this->persistEvent($msg);
|
||||||
|
|
@ -382,7 +396,9 @@ class NextcloudImportService
|
||||||
$ch = curl_init($url);
|
$ch = curl_init($url);
|
||||||
curl_setopt($ch, CURLOPT_USERPWD, $this->user . ':' . $this->pass);
|
curl_setopt($ch, CURLOPT_USERPWD, $this->user . ':' . $this->pass);
|
||||||
curl_setopt($ch, CURLOPT_RETURNTRANSFER, true);
|
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_TIMEOUT, 60);
|
||||||
curl_setopt($ch, CURLOPT_SSL_VERIFYPEER, true);
|
curl_setopt($ch, CURLOPT_SSL_VERIFYPEER, true);
|
||||||
return $ch;
|
return $ch;
|
||||||
|
|
|
||||||
|
|
@ -315,7 +315,9 @@
|
||||||
<label>Nextcloud-URL</label>
|
<label>Nextcloud-URL</label>
|
||||||
<input type="url" name="nextcloud_url"
|
<input type="url" name="nextcloud_url"
|
||||||
value="<?= $h($settings['nextcloud_url'] ?? '') ?>"
|
value="<?= $h($settings['nextcloud_url'] ?? '') ?>"
|
||||||
|
class="<?= isset($errors['nextcloud_url']) ? 'input-error' : '' ?>"
|
||||||
placeholder="https://cloud.example.com">
|
placeholder="https://cloud.example.com">
|
||||||
|
<?php if (isset($errors['nextcloud_url'])): ?><div class="error-text"><?= $h($errors['nextcloud_url']) ?></div><?php endif; ?>
|
||||||
</div>
|
</div>
|
||||||
<div class="form-grid form-grid-2">
|
<div class="form-grid form-grid-2">
|
||||||
<div class="form-group">
|
<div class="form-group">
|
||||||
|
|
|
||||||
84
tests/56_NextcloudSsrfTest.php
Normal file
84
tests/56_NextcloudSsrfTest.php
Normal file
|
|
@ -0,0 +1,84 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
declare(strict_types=1);
|
||||||
|
|
||||||
|
use Steuerkram\Database;
|
||||||
|
use Steuerkram\Services\NextcloudImportService;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Test 56: SSRF-Schutz für nextcloud_url (Issue #127).
|
||||||
|
* Prüft, dass NextcloudImportService die konfigurierte Nextcloud-URL beim
|
||||||
|
* Konstruieren defensiv über SsrfGuard prüft und bei unzulässigen Zielen
|
||||||
|
* (http://, private/loopback/link-local IPs) keine Verbindung aufbaut,
|
||||||
|
* statt einen echten Netzwerk-Request auszulösen.
|
||||||
|
*/
|
||||||
|
class NextcloudSsrfTest extends TestCase
|
||||||
|
{
|
||||||
|
protected string $testName = 'Test 56: SSRF-Schutz für nextcloud_url beim Nextcloud-Belegimport';
|
||||||
|
|
||||||
|
public function run(): void
|
||||||
|
{
|
||||||
|
$this->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);
|
||||||
|
}
|
||||||
|
}
|
||||||
Loading…
Add table
Reference in a new issue