Fix #119: SMTP-/Nextcloud-Passwörter verschlüsselt gespeichert, kein Klartext im HTML mehr

This commit is contained in:
fds2610-bot 2026-07-08 07:48:16 +02:00
parent f6eb71ddd7
commit 6292c90fc5
6 changed files with 97 additions and 6 deletions

View file

@ -2,6 +2,15 @@
---
## 2026-07-08 – Fix: SMTP-/Nextcloud-Zugangsdaten im Klartext gespeichert und im HTML sichtbar (Issue #119)
- **Hoch:** `mail_smtp_pass` (SMTP-Passwort) und `nextcloud_pass` (Nextcloud-App-Passwort) wurden unverschlüsselt in `company_settings` abgelegt, obwohl für vergleichbar sensible Felder (FinTS-PIN, Client-ID/Secret) bereits `Crypto::encrypt()`/`Crypto::decrypt()` (AES-256-GCM) etabliert ist. Zusätzlich schrieb `src/Views/settings/form.php` beide Klartext-Passwörter in das `value`-Attribut zurück, wodurch sie für jeden Betrachter des Seitenquelltexts sichtbar waren.
- **`src/Models/Setting.php`:** `get()` entschlüsselt `nextcloud_pass`/`mail_smtp_pass` nun über `Crypto::decrypt()` (Migrationsmodus: bestehende Klartextwerte in der DB werden unverändert durchgereicht), `update()` verschlüsselt beide Felder vor dem Speichern über `Crypto::encrypt()` – analog zum bestehenden Muster in `BankSource.php`.
- **`src/Controllers/SettingsController.php`:** `update()` behält bei leer gelassenem Passwortfeld das bisherige (entschlüsselte) Passwort bei, statt es zu überschreiben – analog zum bestehenden Muster in `BankController`.
- **`src/Views/settings/form.php`:** `value`-Attribute der beiden Passwortfelder entfernt; stattdessen Platzhalter „(gespeichert – leer lassen für unverändert)", wenn bereits ein Wert existiert.
- **`migrations/085_encrypt_settings_passwords.sql`:** `mail_smtp_pass` von `VARCHAR(255)` auf `TEXT` erweitert, damit der Verschlüsselungs-Overhead (Prefix + Base64) nicht abgeschnitten wird.
- **`tests/52_SettingsPasswordEncryptionTest.php`:** Neuer Test prüft, dass beide Felder verschlüsselt in der DB landen, über `Setting::get()` korrekt entschlüsselt werden und bei leerem Wert nicht verschlüsselt gespeichert werden.
## 2026-07-08 – Fix: SSRF über ungeprüftes fints_url-Feld bei Bankverbindungen (Issue #118)
- **Kritisch:** Beim Anlegen/Bearbeiten einer FinTS-Bankverbindung wurde die vom Benutzer eingegebene `fints_url` ungefiltert übernommen und später von `FinTsFetchService` direkt als Ziel-URL für eine echte HTTP-Verbindung verwendet – ein authentifizierter Benutzer konnte den Server so Requests gegen beliebige Ziele absetzen lassen (interne Dienste, `localhost`, Cloud-Metadata-Endpunkte, CWE-918).

View file

@ -0,0 +1,5 @@
-- Fix #119: mail_smtp_pass und nextcloud_pass werden nun verschlüsselt (Crypto::encrypt)
-- abgelegt. mail_smtp_pass wird auf TEXT erweitert, damit der Verschlüsselungs-Overhead
-- (enc:v1: Prefix + Base64 aus IV/Tag/Ciphertext) nicht abgeschnitten wird.
ALTER TABLE company_settings
MODIFY COLUMN mail_smtp_pass TEXT NOT NULL;

View file

@ -39,6 +39,13 @@ class SettingsController extends BaseController
if (empty($data['plz'])) $errors['plz'] = 'Pflichtfeld';
if (empty($data['ort'])) $errors['ort'] = 'Pflichtfeld';
// Passwortfelder: leer gelassen = bestehendes (verschlüsseltes) Passwort behalten
$current = Setting::get();
foreach (['nextcloud_pass', 'mail_smtp_pass'] as $pwField) {
$newVal = trim($data[$pwField] ?? '');
$data[$pwField] = $newVal !== '' ? $newVal : ($current[$pwField] ?? '');
}
if ($errors) {
$vf = dirname(__DIR__) . '/version.txt';
$version = file_exists($vf) ? trim(file_get_contents($vf)) : '';

View file

@ -5,12 +5,18 @@ declare(strict_types=1);
namespace Steuerkram\Models;
use Steuerkram\Database;
use Steuerkram\Services\Crypto;
class Setting
{
public static function get(): array
{
return Database::fetchOne('SELECT * FROM company_settings WHERE id = 1') ?: [];
$row = Database::fetchOne('SELECT * FROM company_settings WHERE id = 1') ?: [];
if ($row) {
$row['nextcloud_pass'] = Crypto::decrypt((string)($row['nextcloud_pass'] ?? ''));
$row['mail_smtp_pass'] = Crypto::decrypt((string)($row['mail_smtp_pass'] ?? ''));
}
return $row;
}
public static function isTestMode(): bool
@ -70,14 +76,14 @@ class Setting
isset($data['ki_vorschlag_aktiv']) ? 1 : 0,
$data['nextcloud_url'] ?: null,
$data['nextcloud_user'] ?: null,
$data['nextcloud_pass'] ?: null,
!empty($data['nextcloud_pass']) ? Crypto::encrypt($data['nextcloud_pass']) : null,
$data['nextcloud_folder'] ?: '/Belege/Import',
$data['nextcloud_done'] ?: null,
$data['mail_smtp_host'] ?: '',
(int)($data['mail_smtp_port'] ?? 587),
in_array($data['mail_smtp_encryption'] ?? '', ['tls', 'ssl', 'none']) ? $data['mail_smtp_encryption'] : 'tls',
$data['mail_smtp_user'] ?: '',
$data['mail_smtp_pass'] ?: '',
!empty($data['mail_smtp_pass']) ? Crypto::encrypt($data['mail_smtp_pass']) : '',
$data['mail_from_name'] ?: '',
$data['mail_from_email'] ?: '',
$data['mail_subject_template'] ?: 'Rechnung %RNR%',

View file

@ -327,9 +327,8 @@
<div class="form-group">
<label>App-Passwort</label>
<input type="password" name="nextcloud_pass"
value="<?= $h($settings['nextcloud_pass'] ?? '') ?>"
autocomplete="new-password"
placeholder="<?= !empty($settings['nextcloud_pass']) ? '(gespeichert)' : '' ?>">
placeholder="<?= !empty($settings['nextcloud_pass']) ? '(gespeichert – leer lassen für unverändert)' : '' ?>">
<div class="form-hint">Empfohlen: App-Passwort aus Nextcloud → Einstellungen → Sicherheit</div>
</div>
</div>
@ -381,7 +380,8 @@
</div>
<div class="form-group">
<label>SMTP-Passwort</label>
<input type="password" name="mail_smtp_pass" value="<?= $h($settings['mail_smtp_pass'] ?? '') ?>" autocomplete="new-password">
<input type="password" name="mail_smtp_pass" autocomplete="new-password"
placeholder="<?= !empty($settings['mail_smtp_pass']) ? '(gespeichert – leer lassen für unverändert)' : '' ?>">
</div>
</div>
<div class="form-grid form-grid-2">

View file

@ -0,0 +1,64 @@
<?php
declare(strict_types=1);
use Steuerkram\Config;
use Steuerkram\Database;
use Steuerkram\Models\Setting;
/**
* Test 52: nextcloud_pass und mail_smtp_pass werden verschlüsselt gespeichert
* (Fix #119) und über Setting::get() wieder korrekt entschlüsselt.
*/
class SettingsPasswordEncryptionTest extends TestCase
{
protected string $testName = 'Test 52: Settings – Passwort-Verschlüsselung (Nextcloud/SMTP)';
public function run(): void
{
$oldKey = $_ENV['APP_KEY'] ?? null;
if (empty(Config::get('app_key', ''))) {
$_ENV['APP_KEY'] = str_repeat('c3', 32);
Config::load();
}
try {
$before = Setting::get();
Setting::update(array_merge($before, [
'firma' => $before['firma'] ?: 'Testfirma',
'strasse' => $before['strasse'] ?: 'Teststr. 1',
'plz' => $before['plz'] ?: '12345',
'ort' => $before['ort'] ?: 'Testort',
'nextcloud_pass' => 'geheimes-nc-passwort',
'mail_smtp_pass' => 'geheimes-smtp-passwort',
]));
// Roh-DB-Spalte muss verschlüsselt sein, nicht im Klartext
$raw = Database::fetchOne('SELECT nextcloud_pass, mail_smtp_pass FROM company_settings WHERE id = 1');
$this->assert(str_starts_with($raw['nextcloud_pass'], 'enc:v1:'), 'nextcloud_pass ist in der DB verschlüsselt');
$this->assert(str_starts_with($raw['mail_smtp_pass'], 'enc:v1:'), 'mail_smtp_pass ist in der DB verschlüsselt');
// Setting::get() liefert wieder den Klartext
$after = Setting::get();
$this->assertEquals('geheimes-nc-passwort', $after['nextcloud_pass'], 'nextcloud_pass entschlüsselt korrekt');
$this->assertEquals('geheimes-smtp-passwort', $after['mail_smtp_pass'], 'mail_smtp_pass entschlüsselt korrekt');
// Leerer Wert wird nicht "verschlüsselt" (Crypto::encrypt('') === '') und bleibt leer/null
Setting::update(array_merge($after, [
'nextcloud_pass' => '',
'mail_smtp_pass' => '',
]));
$rawEmpty = Database::fetchOne('SELECT nextcloud_pass, mail_smtp_pass FROM company_settings WHERE id = 1');
$this->assert(empty($rawEmpty['nextcloud_pass']), 'nextcloud_pass wird bei leerem Wert nicht verschlüsselt gespeichert');
$this->assert(empty($rawEmpty['mail_smtp_pass']), 'mail_smtp_pass wird bei leerem Wert nicht verschlüsselt gespeichert');
} finally {
if ($oldKey === null) {
unset($_ENV['APP_KEY']);
} else {
$_ENV['APP_KEY'] = $oldKey;
}
Config::load();
}
}
}