From 2012f228c4779cbdd72fad0b35caf4bf9efde6d8 Mon Sep 17 00:00:00 2001 From: fds2610-bot Date: Mon, 6 Jul 2026 23:57:37 +0200 Subject: [PATCH 1/5] =?UTF-8?q?Fix=20#114:=20Beleg-L=C3=B6schung=20nicht?= =?UTF-8?q?=20mehr=20durch=20Datei-Permission-Fehler=20unterbrochen?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Changes.md | 6 ++++ src/Models/Beleg.php | 6 ++-- tests/49_BelegDeleteUnlinkFailTest.php | 42 ++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 2 deletions(-) create mode 100644 tests/49_BelegDeleteUnlinkFailTest.php diff --git a/Changes.md b/Changes.md index bab3732..28e6c30 100644 --- a/Changes.md +++ b/Changes.md @@ -2,6 +2,12 @@ --- +## 2026-07-06 – Fix: Fehlermeldung beim Beleg-Löschen, falls Datei nicht löschbar ist (Issue #114) + +- **`src/Models/Beleg.php`:** `delete()` nutzte ein ungeschütztes `unlink()`; schlug dieses auf dem Server mit „Permission denied" fehl, wurde eine PHP-Warning ausgegeben, die den nachfolgenden Redirect-Header zerstörte („headers already sent"). Der Datensatz wurde dabei trotzdem gelöscht, was zu Verwirrung führte. Fehler beim Datei-Löschen wird jetzt abgefangen und nur noch geloggt (`error_log`), die Löschung des Datensatzes läuft unverändert durch. + +--- + ## 2026-07-02 – Feature: Zusätzliche CC-E-Mail-Adressen pro Kunde (Issue #111) - **`migrations/084_customer_cc_emails.sql`:** Neue Spalten `email_cc1`, `email_cc2` in `customers` diff --git a/src/Models/Beleg.php b/src/Models/Beleg.php index 56781c7..edaa37c 100644 --- a/src/Models/Beleg.php +++ b/src/Models/Beleg.php @@ -249,9 +249,11 @@ class Beleg $cnt = Database::fetchOne('SELECT COUNT(*) as cnt FROM bookings WHERE beleg_id=?', [$id]); if ($cnt && $cnt['cnt'] > 0) return false; - // Delete file + // Delete file (Datei-Löschung darf DB-Löschung nicht durch Warnings/Header-Fehler stören) $fullPath = dirname(__DIR__, 2) . '/public/' . $beleg['storage_path']; - if (file_exists($fullPath)) unlink($fullPath); + if (file_exists($fullPath) && !@unlink($fullPath)) { + error_log('Beleg::delete: Datei konnte nicht gelöscht werden: ' . $fullPath); + } Database::execute('DELETE FROM belege WHERE id=?', [$id]); return true; diff --git a/tests/49_BelegDeleteUnlinkFailTest.php b/tests/49_BelegDeleteUnlinkFailTest.php new file mode 100644 index 0000000..aff945a --- /dev/null +++ b/tests/49_BelegDeleteUnlinkFailTest.php @@ -0,0 +1,42 @@ + 'BLG-TEST-49', + 'filename' => 'original.pdf', + 'storage_path' => 'static/uploads/test_49_unlink_fail/original.pdf', + 'mime_type' => 'application/pdf', + 'file_hash' => 'testhash49', + 'beleg_datum' => '2026-01-01', + ]); + $this->assertGreaterThan(0, $id, 'Beleg angelegt'); + + $result = Beleg::delete($id); + $this->assert($result === true, 'delete() liefert true, obwohl unlink() fehlschlägt'); + $this->assert(Beleg::find($id) === false, 'Beleg-Datensatz wurde trotz Datei-Fehler aus der DB gelöscht'); + } finally { + chmod($dir, 0755); + if (file_exists($file)) unlink($file); + if (is_dir($dir)) rmdir($dir); + } + } +} From 649684a77b82c07897dc0c06f9301ce3193b1f77 Mon Sep 17 00:00:00 2001 From: fds2610-bot Date: Wed, 8 Jul 2026 07:37:45 +0200 Subject: [PATCH 2/5] Fix #116: Beleg-Uploads aus dem Web-Root entfernt (nach data/uploads verschoben) --- Changes.md | 10 ++++++++++ Dockerfile | 3 +-- docker-compose.yml | 2 +- docker-entrypoint.sh | 2 +- docs/datenbankstruktur.md | 2 +- src/Config.php | 2 +- src/Controllers/BankController.php | 7 ++++--- src/Controllers/BelegController.php | 13 +++++++------ src/Controllers/InvoiceController.php | 19 ++++++++++--------- src/Models/Beleg.php | 15 +++++++++++++-- src/Services/KontierungsvorschlagService.php | 2 +- src/Services/NextcloudImportService.php | 7 ++++--- tests/49_BelegDeleteUnlinkFailTest.php | 4 ++-- 13 files changed, 56 insertions(+), 32 deletions(-) diff --git a/Changes.md b/Changes.md index 28e6c30..74f4cc1 100644 --- a/Changes.md +++ b/Changes.md @@ -2,6 +2,16 @@ --- +## 2026-07-08 – Fix: Unauthentifizierter Direktzugriff auf Belege/Rechnungs-PDFs (Issue #116) + +- **Kritisch:** Hochgeladene Belege und Rechnungs-PDFs lagen unter `public/static/uploads/...` im Web-Root und wurden von Apache direkt ausgeliefert, bevor der Request `index.php`/`Auth::require()` erreichte – ein Angreifer konnte Belege ohne Anmeldung per URL abrufen. +- **`src/Config.php`:** `upload_dir` zeigt jetzt auf `data/uploads` (außerhalb des Web-Roots, analog zum bereits per Apache gesperrten `/data`-Verzeichnis) +- **`src/Models/Beleg.php`:** Neue Methode `absolutePath()` als einzige Quelle für die Pfad-Auflösung; unterstützt weiterhin Bestandsdaten mit dem alten Präfix `static/uploads/` in `storage_path` +- **`src/Controllers/BelegController.php`, `InvoiceController.php`, `BankController.php`, `src/Services/NextcloudImportService.php`, `KontierungsvorschlagService.php`:** Alle Stellen, die Beleg-Dateien schreiben oder lesen, nutzen jetzt `Config::get('upload_dir')` bzw. `Beleg::absolutePath()` statt eigener `public/static/uploads`-Pfade +- **`docker-compose.yml`, `Dockerfile`, `docker-entrypoint.sh`:** Upload-Verzeichnis wird nach `/var/www/html/data/uploads` gemountet/angelegt (bestehender Host-Ordner `./uploads` bleibt unverändert) + +--- + ## 2026-07-06 – Fix: Fehlermeldung beim Beleg-Löschen, falls Datei nicht löschbar ist (Issue #114) - **`src/Models/Beleg.php`:** `delete()` nutzte ein ungeschütztes `unlink()`; schlug dieses auf dem Server mit „Permission denied" fehl, wurde eine PHP-Warning ausgegeben, die den nachfolgenden Redirect-Header zerstörte („headers already sent"). Der Datensatz wurde dabei trotzdem gelöscht, was zu Verwirrung führte. Fehler beim Datei-Löschen wird jetzt abgefangen und nur noch geloggt (`error_log`), die Löschung des Datensatzes läuft unverändert durch. diff --git a/Dockerfile b/Dockerfile index f6f169d..0e049c6 100644 --- a/Dockerfile +++ b/Dockerfile @@ -36,9 +36,8 @@ COPY apache/steuerkram.conf /etc/apache2/sites-available/000-default.conf RUN a2enmod rewrite headers # Permissions -RUN mkdir -p /var/www/html/public/static/uploads /var/www/html/data \ +RUN mkdir -p /var/www/html/data/uploads \ && chown -R www-data:www-data /var/www/html \ - && chmod -R 755 /var/www/html/public/static/uploads \ && chmod -R 755 /var/www/html/data EXPOSE 80 diff --git a/docker-compose.yml b/docker-compose.yml index 848f2bf..2acba01 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -7,7 +7,7 @@ services: - ./public:/var/www/html/public - ./src:/var/www/html/src - ./migrations:/var/www/html/migrations - - ./uploads:/var/www/html/public/static/uploads + - ./uploads:/var/www/html/data/uploads - ./data:/var/www/html/data - ./tests:/var/www/html/tests - ./scripts:/var/www/html/scripts diff --git a/docker-entrypoint.sh b/docker-entrypoint.sh index 360f83a..aed7b43 100644 --- a/docker-entrypoint.sh +++ b/docker-entrypoint.sh @@ -1,6 +1,6 @@ #!/bin/sh # Fix ownership of volume-mounted upload directory at container start -chown -R www-data:www-data /var/www/html/public/static/uploads 2>/dev/null || true +chown -R www-data:www-data /var/www/html/data/uploads 2>/dev/null || true # Datenbank-Migrationen automatisch einspielen php /var/www/html/scripts/migrate.php diff --git a/docs/datenbankstruktur.md b/docs/datenbankstruktur.md index b823d68..e80bfb9 100644 --- a/docs/datenbankstruktur.md +++ b/docs/datenbankstruktur.md @@ -116,7 +116,7 @@ MariaDB 11, Engine InnoDB, Zeichensatz `utf8mb4_unicode_ci`. Das Schema wird dur | `id` | INT UNSIGNED PK | | | `beleg_nr` | VARCHAR(30) UNIQUE | Automatisch vergeben (z.B. `BLG-2026-0001`) | | `filename` | VARCHAR(500) | Originaldateiname | -| `storage_path` | VARCHAR(500) | Pfad unter `public/static/uploads/` | +| `storage_path` | VARCHAR(500) | Relativer Pfad unter `data/uploads/` (außerhalb des Web-Roots, Bestandsdaten mit altem Präfix `static/uploads/` werden von `Beleg::absolutePath()` weiter unterstützt) | | `mime_type` | VARCHAR(100) | MIME-Typ der Datei | | `file_hash` | VARCHAR(64) | SHA-256 für GoBD-Integritätsprüfung | | `beleg_datum` | DATE | Datum auf dem Dokument | diff --git a/src/Config.php b/src/Config.php index 5b36f7e..c43a6c6 100644 --- a/src/Config.php +++ b/src/Config.php @@ -17,7 +17,7 @@ class Config 'db_pass' => $_ENV['DB_PASS'] ?? getenv('DB_PASS') ?: 'steuerkram', 'app_key' => $_ENV['APP_KEY'] ?? getenv('APP_KEY') ?: '', 'sidebar_bg_color' => $_ENV['SIDEBAR_BG_COLOR'] ?? getenv('SIDEBAR_BG_COLOR') ?: '#1e293b', - 'upload_dir' => dirname(__DIR__) . '/public/static/uploads', + 'upload_dir' => dirname(__DIR__) . '/data/uploads', 'base_url' => '', ]; } diff --git a/src/Controllers/BankController.php b/src/Controllers/BankController.php index 37b65a5..7f962ef 100644 --- a/src/Controllers/BankController.php +++ b/src/Controllers/BankController.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Steuerkram\Controllers; +use Steuerkram\Config; use Steuerkram\Models\BankProvider; use Steuerkram\Models\BankSource; use Steuerkram\Models\BankTransaction; @@ -350,12 +351,12 @@ class BankController extends BaseController $ext = $allowedMime[$mime]; $belegNr = Beleg::generateBelegNr(); $year = date('Y', strtotime($transaction['wertstellung'])); - $relDir = "static/uploads/{$year}/{$belegNr}"; - $absDir = dirname(__DIR__, 2) . "/public/{$relDir}"; + $relDir = "{$year}/{$belegNr}"; + $absDir = Config::get('upload_dir') . "/{$relDir}"; if (!is_dir($absDir)) mkdir($absDir, 0755, true); $storagePath = "{$relDir}/original.{$ext}"; - $absPath = dirname(__DIR__, 2) . "/public/{$storagePath}"; + $absPath = Config::get('upload_dir') . "/{$storagePath}"; move_uploaded_file($file['tmp_name'], $absPath); $hash = hash_file('sha256', $absPath); diff --git a/src/Controllers/BelegController.php b/src/Controllers/BelegController.php index eb98f58..23b8ad8 100644 --- a/src/Controllers/BelegController.php +++ b/src/Controllers/BelegController.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Steuerkram\Controllers; +use Steuerkram\Config; use Steuerkram\Models\Beleg; use Steuerkram\Models\Booking; use Steuerkram\Models\Account; @@ -159,14 +160,14 @@ class BelegController extends BaseController : date('Y'); // Store under uploads/{year}/{belegNr}/original.{ext} - $relDir = "static/uploads/{$year}/{$belegNr}"; - $absDir = dirname(__DIR__, 2) . "/public/{$relDir}"; + $relDir = "{$year}/{$belegNr}"; + $absDir = Config::get('upload_dir') . "/{$relDir}"; if (!is_dir($absDir)) mkdir($absDir, 0755, true); $filename = pathinfo($file['name'], PATHINFO_BASENAME); $filenameWithoutExt = pathinfo($file['name'], PATHINFO_FILENAME); $storagePath = "{$relDir}/original.{$ext}"; - $absPath = dirname(__DIR__, 2) . "/public/{$storagePath}"; + $absPath = Config::get('upload_dir') . "/{$storagePath}"; move_uploaded_file($file['tmp_name'], $absPath); @@ -311,8 +312,8 @@ class BelegController extends BaseController exit; } - $basePath = (string)realpath(dirname(__DIR__, 2) . '/public/static/uploads'); - $absPath = dirname(__DIR__, 2) . '/public/' . $beleg['storage_path']; + $basePath = (string)realpath(Config::get('upload_dir')); + $absPath = Beleg::absolutePath($beleg); $realPath = realpath($absPath); if ($realPath === false || !str_starts_with($realPath, $basePath)) { @@ -466,7 +467,7 @@ class BelegController extends BaseController return; } - $absPath = dirname(__DIR__, 2) . '/public/' . $beleg['storage_path']; + $absPath = Beleg::absolutePath($beleg); if (!file_exists($absPath)) { http_response_code(404); echo json_encode(['error' => 'Datei nicht gefunden.']); diff --git a/src/Controllers/InvoiceController.php b/src/Controllers/InvoiceController.php index 72d9384..b5187fe 100644 --- a/src/Controllers/InvoiceController.php +++ b/src/Controllers/InvoiceController.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Steuerkram\Controllers; +use Steuerkram\Config; use Steuerkram\Models\Invoice; use Steuerkram\Models\Customer; use Steuerkram\Models\Beleg; @@ -291,8 +292,8 @@ class InvoiceController extends BaseController $beleg = null; if (!empty($invoice['beleg_id'])) { $beleg = Beleg::find((int)$invoice['beleg_id']); - $absPath = $beleg - ? dirname(__DIR__, 2) . '/public/' . $beleg['storage_path'] + $absPath = ($beleg && !empty($beleg['storage_path'])) + ? Beleg::absolutePath($beleg) : ''; if ($absPath && is_file($absPath)) { header('Content-Type: application/pdf'); @@ -322,17 +323,17 @@ class InvoiceController extends BaseController if ($invoice['status'] !== 'Entwurf' && $beleg !== null) { if (!empty($beleg['storage_path'])) { $relPath = $beleg['storage_path']; - $absDir = dirname(dirname(__DIR__, 2) . '/public/' . $relPath); + $absDir = dirname(Beleg::absolutePath($beleg)); } else { $year = date('Y', strtotime($invoice['rechnungs_datum'])); - $relDir = 'static/uploads/' . $year . '/' . $beleg['beleg_nr']; + $relDir = $year . '/' . $beleg['beleg_nr']; $relPath = $relDir . '/original.pdf'; - $absDir = dirname(__DIR__, 2) . '/public/' . $relDir; + $absDir = Config::get('upload_dir') . '/' . $relDir; } if (!is_dir($absDir)) { mkdir($absDir, 0755, true); } - $absFilePath = dirname(__DIR__, 2) . '/public/' . $relPath; + $absFilePath = Beleg::absolutePath(['storage_path' => $relPath]); file_put_contents($absFilePath, $pdf); \Steuerkram\Database::execute( 'UPDATE belege SET storage_path=?, filename=?, file_hash=?, updated_at=NOW() WHERE id=?', @@ -512,13 +513,13 @@ class InvoiceController extends BaseController $belegNr = Beleg::generateBelegNr(); $year = date('Y', strtotime($invoice['rechnungs_datum'])); - $relDir = "static/uploads/{$year}/{$belegNr}"; - $absDir = dirname(__DIR__, 2) . "/public/{$relDir}"; + $relDir = "{$year}/{$belegNr}"; + $absDir = Config::get('upload_dir') . "/{$relDir}"; if (!is_dir($absDir)) mkdir($absDir, 0755, true); $filename = $invoice['rechnungs_nr'] . '.pdf'; $storagePath = "{$relDir}/original.pdf"; - file_put_contents(dirname(__DIR__, 2) . "/public/{$storagePath}", $pdf); + file_put_contents(Config::get('upload_dir') . "/{$storagePath}", $pdf); $belegId = Beleg::create([ 'invoice_id' => $id, diff --git a/src/Models/Beleg.php b/src/Models/Beleg.php index edaa37c..c0362d1 100644 --- a/src/Models/Beleg.php +++ b/src/Models/Beleg.php @@ -4,10 +4,21 @@ declare(strict_types=1); namespace Steuerkram\Models; +use Steuerkram\Config; use Steuerkram\Database; class Beleg { + // Bestandsdaten mit altem, webroot-relativem Präfix "static/uploads/" bleiben lesbar. + public static function absolutePath(array $beleg): string + { + $relPath = (string)($beleg['storage_path'] ?? ''); + if (str_starts_with($relPath, 'static/uploads/')) { + $relPath = substr($relPath, strlen('static/uploads/')); + } + return Config::get('upload_dir') . '/' . $relPath; + } + public static function all(array $filter = [], int $page = 1, int $perPage = 30): array { $where = ['1=1']; @@ -250,7 +261,7 @@ class Beleg if ($cnt && $cnt['cnt'] > 0) return false; // Delete file (Datei-Löschung darf DB-Löschung nicht durch Warnings/Header-Fehler stören) - $fullPath = dirname(__DIR__, 2) . '/public/' . $beleg['storage_path']; + $fullPath = self::absolutePath($beleg); if (file_exists($fullPath) && !@unlink($fullPath)) { error_log('Beleg::delete: Datei konnte nicht gelöscht werden: ' . $fullPath); } @@ -300,7 +311,7 @@ class Beleg $beleg = self::find($id); if (!$beleg) return false; - $fullPath = dirname(__DIR__, 2) . '/public/' . $beleg['storage_path']; + $fullPath = self::absolutePath($beleg); if (!file_exists($fullPath)) return false; return hash_file('sha256', $fullPath) === $beleg['file_hash']; diff --git a/src/Services/KontierungsvorschlagService.php b/src/Services/KontierungsvorschlagService.php index 27a6dc7..ef15353 100644 --- a/src/Services/KontierungsvorschlagService.php +++ b/src/Services/KontierungsvorschlagService.php @@ -26,7 +26,7 @@ class KontierungsvorschlagService public function extractText(array $beleg): string { - $absPath = dirname(__DIR__, 2) . '/public/' . $beleg['storage_path']; + $absPath = Beleg::absolutePath($beleg); if (!file_exists($absPath)) return ''; $mime = strtolower($beleg['mime_type'] ?? ''); diff --git a/src/Services/NextcloudImportService.php b/src/Services/NextcloudImportService.php index 4afd127..8f0d1b3 100644 --- a/src/Services/NextcloudImportService.php +++ b/src/Services/NextcloudImportService.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Steuerkram\Services; +use Steuerkram\Config; use Steuerkram\Database; use Steuerkram\Models\Beleg; use Steuerkram\Models\EventLog; @@ -260,8 +261,8 @@ class NextcloudImportService // In Upload-Verzeichnis ablegen $belegNr = Beleg::generateBelegNr(); $year = date('Y'); - $relDir = "static/uploads/{$year}/{$belegNr}"; - $absDir = dirname(__DIR__, 2) . "/public/{$relDir}"; + $relDir = "{$year}/{$belegNr}"; + $absDir = Config::get('upload_dir') . "/{$relDir}"; if (!is_dir($absDir) && !mkdir($absDir, 0755, true)) { $msg = "FEHLER: Verzeichnis konnte nicht erstellt werden: {$absDir}"; @@ -273,7 +274,7 @@ class NextcloudImportService } $storagePath = "{$relDir}/original.{$ext}"; - $absPath = dirname(__DIR__, 2) . "/public/{$storagePath}"; + $absPath = Config::get('upload_dir') . "/{$storagePath}"; if (!rename($tmpPath, $absPath)) { $msg = "FEHLER: Datei konnte nicht verschoben werden: {$filename}"; diff --git a/tests/49_BelegDeleteUnlinkFailTest.php b/tests/49_BelegDeleteUnlinkFailTest.php index aff945a..fb7947f 100644 --- a/tests/49_BelegDeleteUnlinkFailTest.php +++ b/tests/49_BelegDeleteUnlinkFailTest.php @@ -10,7 +10,7 @@ class BelegDeleteUnlinkFailTest extends TestCase public function run(): void { - $dir = dirname(__DIR__) . '/public/static/uploads/test_49_unlink_fail'; + $dir = \Steuerkram\Config::get('upload_dir') . '/test_49_unlink_fail'; $file = $dir . '/original.pdf'; mkdir($dir, 0755, true); @@ -23,7 +23,7 @@ class BelegDeleteUnlinkFailTest extends TestCase $id = Beleg::create([ 'beleg_nr' => 'BLG-TEST-49', 'filename' => 'original.pdf', - 'storage_path' => 'static/uploads/test_49_unlink_fail/original.pdf', + 'storage_path' => 'test_49_unlink_fail/original.pdf', 'mime_type' => 'application/pdf', 'file_hash' => 'testhash49', 'beleg_datum' => '2026-01-01', From d57f334229f92fdda39cd2e8315dcd0d546bd9fe Mon Sep 17 00:00:00 2001 From: fds2610-bot Date: Wed, 8 Jul 2026 07:41:44 +0200 Subject: [PATCH 3/5] =?UTF-8?q?Fix=20#117:=20E-Mail-Header-/SMTP-Command-I?= =?UTF-8?q?njection=20=C3=BCber=20Empf=C3=A4nger-/CC-Adressen=20verhindert?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Changes.md | 10 ++++ src/Controllers/CustomerController.php | 8 +++ src/Services/SmtpMailer.php | 6 +++ src/Views/customers/form.php | 10 ++-- tests/50_EmailHeaderInjectionTest.php | 74 ++++++++++++++++++++++++++ 5 files changed, 105 insertions(+), 3 deletions(-) create mode 100644 tests/50_EmailHeaderInjectionTest.php diff --git a/Changes.md b/Changes.md index 74f4cc1..0ae2aa4 100644 --- a/Changes.md +++ b/Changes.md @@ -2,6 +2,16 @@ --- +## 2026-07-08 – Fix: SMTP-/E-Mail-Header-Injection über ungeprüfte Empfänger-/CC-Adressen (Issue #117) + +- **Kritisch:** Empfänger- und CC-Adressen (Rechnungsversand, Test-Mail, Kundenstammdaten) wurden nur `trim()`t, aber nie auf gültiges Format oder enthaltene `\r`/`\n` geprüft. Da `SmtpMailer` das SMTP-Protokoll über einen rohen Socket implementiert, konnte eine Adresse mit eingebettetem CRLF zusätzliche `RCPT TO:`-Kommandos oder Mail-Header einschleusen (CWE-93, SMTP-/Header-Injection). +- **`src/Services/SmtpMailer.php`:** `send()` validiert jetzt jede Empfängeradresse mit `filter_var(..., FILTER_VALIDATE_EMAIL)`, bevor überhaupt eine SMTP-Verbindung aufgebaut wird; ungültige Adressen (inkl. CRLF-Payloads) werfen eine `RuntimeException`. Da alle Versandwege (`EmailService::sendInvoice()`, `sendTestMail()`) durch diese einzige Methode laufen, ist die Prüfung zentral wirksam. +- **`src/Controllers/CustomerController.php`:** Formularvalidierung prüft `email`, `email_cc1`, `email_cc2` zusätzlich mit `filter_var(FILTER_VALIDATE_EMAIL)`, damit gar nicht erst ungültige Adressen in der Kundendatenbank landen. +- **`src/Views/customers/form.php`:** Fehleranzeige für die drei E-Mail-Felder ergänzt. +- **`tests/50_EmailHeaderInjectionTest.php`:** Neuer Test prüft, dass CRLF-Injection-Payloads und ungültige Adressformate von `SmtpMailer::send()` abgelehnt werden. + +--- + ## 2026-07-08 – Fix: Unauthentifizierter Direktzugriff auf Belege/Rechnungs-PDFs (Issue #116) - **Kritisch:** Hochgeladene Belege und Rechnungs-PDFs lagen unter `public/static/uploads/...` im Web-Root und wurden von Apache direkt ausgeliefert, bevor der Request `index.php`/`Auth::require()` erreichte – ein Angreifer konnte Belege ohne Anmeldung per URL abrufen. diff --git a/src/Controllers/CustomerController.php b/src/Controllers/CustomerController.php index 1399ec1..af0cb51 100644 --- a/src/Controllers/CustomerController.php +++ b/src/Controllers/CustomerController.php @@ -139,6 +139,14 @@ class CustomerController extends BaseController if (empty($data['strasse'])) $errors['strasse'] = 'Pflichtfeld'; if (empty($data['plz'])) $errors['plz'] = 'Pflichtfeld'; if (empty($data['ort'])) $errors['ort'] = 'Pflichtfeld'; + + foreach (['email', 'email_cc1', 'email_cc2'] as $field) { + $value = trim($data[$field] ?? ''); + if ($value !== '' && filter_var($value, FILTER_VALIDATE_EMAIL) === false) { + $errors[$field] = 'Ungültige E-Mail-Adresse'; + } + } + return $errors; } } diff --git a/src/Services/SmtpMailer.php b/src/Services/SmtpMailer.php index 494b136..b54b493 100644 --- a/src/Services/SmtpMailer.php +++ b/src/Services/SmtpMailer.php @@ -39,6 +39,12 @@ class SmtpMailer ?string $attachContent = null, ?string $attachName = null, ): void { + foreach ($to as $addr) { + if (filter_var($addr, FILTER_VALIDATE_EMAIL) === false) { + throw new \RuntimeException('Ungültige E-Mail-Adresse: ' . $addr); + } + } + $this->connect(); $this->authenticate(); $this->sendMessage($to, $subject, $body, $attachContent, $attachName); diff --git a/src/Views/customers/form.php b/src/Views/customers/form.php index 1a860f3..c14aa4b 100644 --- a/src/Views/customers/form.php +++ b/src/Views/customers/form.php @@ -52,7 +52,9 @@
- + +
@@ -64,12 +66,14 @@
+ placeholder="cc1@example.com" class=""> +
+ placeholder="cc2@example.com" class=""> +

diff --git a/tests/50_EmailHeaderInjectionTest.php b/tests/50_EmailHeaderInjectionTest.php new file mode 100644 index 0000000..80c4329 --- /dev/null +++ b/tests/50_EmailHeaderInjectionTest.php @@ -0,0 +1,74 @@ +send( + ["opfer@example.com\r\nRCPT TO:"], + 'Betreff', + 'Text', + ); + $this->assert(false, 'CRLF-Adresse wirft RuntimeException'); + } catch (\RuntimeException $e) { + $this->assert(true, 'CRLF-Adresse wirft RuntimeException'); + } + + // Zusätzlicher Header via Newline im To-Feld wird abgelehnt + try { + $mailer->send( + ["opfer@example.com\nBcc: angreifer@example.com"], + 'Betreff', + 'Text', + ); + $this->assert(false, 'Header-Injection-Adresse wirft RuntimeException'); + } catch (\RuntimeException $e) { + $this->assert(true, 'Header-Injection-Adresse wirft RuntimeException'); + } + + // Grob fehlerhaftes Format wird abgelehnt + try { + $mailer->send(['keine-email-adresse'], 'Betreff', 'Text'); + $this->assert(false, 'Ungültiges Format wirft RuntimeException'); + } catch (\RuntimeException $e) { + $this->assert(true, 'Ungültiges Format wirft RuntimeException'); + } + + // Kunde: CC-Adressen mit CRLF/ungültigem Format werden vom Model nicht blind übernommen, + // sondern müssen weiterhin filter_var-tauglich sein, bevor sie je in den Mailer gelangen + $this->assert( + filter_var("opfer@example.com\r\nRCPT TO:", FILTER_VALIDATE_EMAIL) === false, + 'filter_var erkennt CRLF-Payload als ungültig' + ); + + // Regressionsschutz: gültige Adressen funktionieren weiterhin (Verbindungsaufbau schlägt + // erwartungsgemäß fehl, da kein echter SMTP-Server erreichbar ist – Validierung selbst greift nicht) + try { + $mailer->send(['gueltig@example.com'], 'Betreff', 'Text'); + $this->assert(false, 'Verbindung zu Dummy-Host sollte fehlschlagen'); + } catch (\RuntimeException $e) { + $this->assert( + str_contains($e->getMessage(), 'SMTP-Verbindung') || !str_contains($e->getMessage(), 'Ungültige E-Mail-Adresse'), + 'Gültige Adresse wird nicht durch Validierung blockiert (Fehler: ' . $e->getMessage() . ')' + ); + } + } +} From f6eb71ddd7f98708d7bf2c5576ad255c834cfe06 Mon Sep 17 00:00:00 2001 From: fds2610-bot Date: Wed, 8 Jul 2026 07:45:08 +0200 Subject: [PATCH 4/5] =?UTF-8?q?Fix=20#118:=20SSRF=20=C3=BCber=20fints=5Fur?= =?UTF-8?q?l-Feld=20bei=20Bankverbindungen=20verhindert?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Changes.md | 10 ++++ docs/funktionen.md | 2 +- src/Controllers/BankController.php | 24 ++++++++- src/Services/FinTsFetchService.php | 7 +++ src/Services/SsrfGuard.php | 86 ++++++++++++++++++++++++++++++ tests/51_SsrfGuardTest.php | 45 ++++++++++++++++ 6 files changed, 171 insertions(+), 3 deletions(-) create mode 100644 src/Services/SsrfGuard.php create mode 100644 tests/51_SsrfGuardTest.php diff --git a/Changes.md b/Changes.md index 0ae2aa4..d5a5ad5 100644 --- a/Changes.md +++ b/Changes.md @@ -2,6 +2,16 @@ --- +## 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). +- **Neu `src/Services/SsrfGuard.php`:** `assertPublicHttpsUrl()` erzwingt Schema `https://`, verbietet Userinfo in der URL und löst den Host auf, um sämtliche zugehörigen IPs gegen private/reservierte Bereiche zu prüfen (RFC1918, Loopback, Link-Local/Metadata via `FILTER_FLAG_NO_PRIV_RANGE`/`FILTER_FLAG_NO_RES_RANGE`); bei Verstoß wirft die Methode eine `RuntimeException`. +- **`src/Controllers/BankController.php`:** `direktStore()` und `direktUpdate()` validieren `fints_url` vor dem Speichern mit `SsrfGuard`; ungültige Ziele werden mit Fehlermeldung abgelehnt statt in der Datenbank zu landen. +- **`src/Services/FinTsFetchService.php`:** `run()` prüft die gespeicherte URL zusätzlich unmittelbar vor dem Verbindungsaufbau erneut (Schutz gegen DNS-Rebinding zwischen Speichern und Abruf). +- **`tests/51_SsrfGuardTest.php`:** Neuer Test prüft, dass `http://`, private/loopback/link-local IP-Literale, `localhost` und URLs mit Userinfo abgelehnt werden, während öffentliche `https://`-Ziele weiterhin akzeptiert werden. + +--- + ## 2026-07-08 – Fix: SMTP-/E-Mail-Header-Injection über ungeprüfte Empfänger-/CC-Adressen (Issue #117) - **Kritisch:** Empfänger- und CC-Adressen (Rechnungsversand, Test-Mail, Kundenstammdaten) wurden nur `trim()`t, aber nie auf gültiges Format oder enthaltene `\r`/`\n` geprüft. Da `SmtpMailer` das SMTP-Protokoll über einen rohen Socket implementiert, konnte eine Adresse mit eingebettetem CRLF zusätzliche `RCPT TO:`-Kommandos oder Mail-Header einschleusen (CWE-93, SMTP-/Header-Injection). diff --git a/docs/funktionen.md b/docs/funktionen.md index 807b2b3..34a29c1 100644 --- a/docs/funktionen.md +++ b/docs/funktionen.md @@ -70,7 +70,7 @@ Anlagenspiegel für abnutzbare Wirtschaftsgüter – siehe [anlagevermögen.md]( - **CSV-Upload**: konfigurierbare Spalten- und Datumsformate je Bank, automatische Format-Erkennung beim Datei-Upload, Vorschau-Panel mit Headerzeile und Spalten-Zuordnung; bei unbekannten Formaten wird ein Vorschlag generiert und auf der Settings-Seite zum expliziten Speichern angeboten - **Neues CSV-Format manuell anlegen**: im „Neue Direktverbindung"-Dialog (Importieren) kann per „+ CSV-Importformat…" ein neues CSV-Format mit Trennzeichen, Datums-/Dezimalformat etc. angelegt werden; Spaltenzuordnung wird anschließend unter Einstellungen → CSV Bankimport Formate verfeinert - **Direktverbindung „PayPal"** (NVP/SOAP-API): API-Benutzer, API-Kennwort, Signature und Sandbox-Flag werden verschlüsselt abgelegt. Live-Log via Server-Sent Events -- **Direktverbindung „FinTS"** (Comdirect, DKB und weitere): BLZ, Benutzerkennung und PIN, optional FinTS-Server-URL (sonst Default aus dem Provider) und IBAN-Filter. PIN wird verschlüsselt abgelegt (AES-256-GCM). Leseabruf mit PIN; TAN-pflichtige Aktionen werden mit Fehlermeldung abgebrochen +- **Direktverbindung „FinTS"** (Comdirect, DKB und weitere): BLZ, Benutzerkennung und PIN, optional FinTS-Server-URL (sonst Default aus dem Provider) und IBAN-Filter. PIN wird verschlüsselt abgelegt (AES-256-GCM). Leseabruf mit PIN; TAN-pflichtige Aktionen werden mit Fehlermeldung abgebrochen. Die FinTS-Server-URL wird beim Speichern und erneut vor jedem Abruf mit `SsrfGuard` geprüft: nur `https://` zu öffentlich erreichbaren Hosts ist erlaubt, private/interne Ziele (RFC1918, Loopback, Link-Local) werden abgelehnt (Schutz vor SSRF) - **Letzter Abrufzeitpunkt** wird pro Verbindung gespeichert; nächster Abruf startet ab 00:00 dieses Tages (max. ein Tag Überlappung für Dedup) - **Abrufen ab**: Im Bearbeiten-Dialog kann pro Direktverbindung ausgewählt werden, ab wann der nächste Lauf Buchungen holt: *Ab letztem Abruf* (Default), *Letzte 7 Tage*, *Aktueller Monat (ab 01.)*, *Letzte 30 Tage* oder *Ohne Datum (letzte 365 Tage)*. Die Sonderauswahl gilt nur einmalig – nach erfolgreichem Abruf fällt die Verbindung wieder auf den Default zurück - **Dublettenprüfung**: CSV-Import vergleicht Datum+Betrag+Text; Direktverbindungen nutzen eine `external_id` (Hash aus IBAN/Datum/Betrag/Verwendungszweck bei FinTS, Transaktions-ID bei PayPal) diff --git a/src/Controllers/BankController.php b/src/Controllers/BankController.php index 7f962ef..d9b295e 100644 --- a/src/Controllers/BankController.php +++ b/src/Controllers/BankController.php @@ -15,6 +15,7 @@ use Steuerkram\Models\Invoice; use Steuerkram\Services\BankImportParser; use Steuerkram\Services\FinTsFetchService; use Steuerkram\Services\PayPalNvpFetchService; +use Steuerkram\Services\SsrfGuard; use Steuerkram\View; class BankController extends BaseController @@ -476,6 +477,16 @@ class BankController extends BaseController $isApi = $provider['verbindungstyp'] === 'API'; $isFints = $provider['verbindungstyp'] === 'FinTS'; + $fintsUrl = $isFints ? (trim($_POST['fints_url'] ?? '') ?: null) : null; + if ($fintsUrl !== null) { + try { + SsrfGuard::assertPublicHttpsUrl($fintsUrl); + } catch (\RuntimeException $e) { + View::redirect('/bank/direkt', 'Ungültige FinTS-URL: ' . $e->getMessage(), 'error'); + return; + } + } + BankSource::create([ 'type' => $isApi ? 'api' : 'fints', 'provider_id' => $providerId, @@ -487,7 +498,7 @@ class BankController extends BaseController 'client_id' => $isApi ? (trim($_POST['client_id'] ?? '') ?: null) : null, 'secret' => $isApi ? (trim($_POST['secret'] ?? '') ?: null) : null, 'blz' => $isFints ? (trim($_POST['blz'] ?? '') ?: null) : null, - 'fints_url' => $isFints ? (trim($_POST['fints_url'] ?? '') ?: null) : null, + 'fints_url' => $fintsUrl, 'benutzerkennung' => $isFints ? (trim($_POST['benutzerkennung'] ?? '') ?: null) : null, 'pin' => $isFints ? (trim($_POST['pin'] ?? '') ?: null) : null, 'tan_mode_id' => $isFints ? (($_POST['tan_mode_id'] ?? '') !== '' ? (int)$_POST['tan_mode_id'] : null) : null, @@ -590,9 +601,18 @@ class BankController extends BaseController ]; } elseif ($isFints) { $newPin = trim($_POST['pin'] ?? ''); + $fintsUrl = trim($_POST['fints_url'] ?? '') ?: null; + if ($fintsUrl !== null) { + try { + SsrfGuard::assertPublicHttpsUrl($fintsUrl); + } catch (\RuntimeException $e) { + View::redirect('/bank/direkt', 'Ungültige FinTS-URL: ' . $e->getMessage(), 'error'); + return; + } + } $data += [ 'blz' => trim($_POST['blz'] ?? '') ?: null, - 'fints_url' => trim($_POST['fints_url'] ?? '') ?: null, + 'fints_url' => $fintsUrl, 'benutzerkennung' => trim($_POST['benutzerkennung'] ?? '') ?: null, 'pin' => $newPin !== '' ? $newPin : $source['pin'], 'tan_mode_id' => ($_POST['tan_mode_id'] ?? '') !== '' ? (int)$_POST['tan_mode_id'] : null, diff --git a/src/Services/FinTsFetchService.php b/src/Services/FinTsFetchService.php index 007dbf8..93190fd 100644 --- a/src/Services/FinTsFetchService.php +++ b/src/Services/FinTsFetchService.php @@ -72,6 +72,13 @@ class FinTsFetchService return; } + try { + SsrfGuard::assertPublicHttpsUrl($url); + } catch (\Throwable $e) { + $log('FEHLER: Unzulässige FinTS-URL – ' . $e->getMessage()); + return; + } + $options = new FinTsOptions(); $options->url = $url; $options->bankCode = $blz; diff --git a/src/Services/SsrfGuard.php b/src/Services/SsrfGuard.php new file mode 100644 index 0000000..c836c24 --- /dev/null +++ b/src/Services/SsrfGuard.php @@ -0,0 +1,86 @@ +assertRejected('http://banking.example.com/fints', 'http:// wird abgelehnt'); + + // IP-Literale in privaten/reservierten Bereichen werden abgelehnt + $this->assertRejected('https://127.0.0.1/fints', 'Loopback-IP (127.0.0.1) wird abgelehnt'); + $this->assertRejected('https://localhost/fints', 'localhost wird abgelehnt'); + $this->assertRejected('https://10.0.0.5/fints', 'RFC1918-IP (10.0.0.5) wird abgelehnt'); + $this->assertRejected('https://192.168.1.1/fints', 'RFC1918-IP (192.168.1.1) wird abgelehnt'); + $this->assertRejected('https://169.254.169.254/fints', 'Link-Local/Metadata-IP (169.254.169.254) wird abgelehnt'); + $this->assertRejected('https://[::1]/fints', 'IPv6-Loopback wird abgelehnt'); + + // Credentials in der URL sind nicht erlaubt (potenzielles Injection-/Redirect-Ziel) + $this->assertRejected('https://user:pass@banking.example.com/fints', 'Userinfo in der URL wird abgelehnt'); + + // Öffentliche IP-Literale und Hostnamen mit https werden akzeptiert + try { + SsrfGuard::assertPublicHttpsUrl('https://8.8.8.8/fints'); + $this->assert(true, 'Öffentliche IP-Literal (8.8.8.8) wird akzeptiert'); + } catch (\RuntimeException $e) { + $this->assert(false, 'Öffentliche IP-Literal (8.8.8.8) wird akzeptiert (Fehler: ' . $e->getMessage() . ')'); + } + } + + private function assertRejected(string $url, string $message): void + { + try { + SsrfGuard::assertPublicHttpsUrl($url); + $this->assert(false, $message); + } catch (\RuntimeException $e) { + $this->assert(true, $message); + } + } +} From 6292c90fc5ce0fde0a1e667b9679f61b2b17dfd8 Mon Sep 17 00:00:00 2001 From: fds2610-bot Date: Wed, 8 Jul 2026 07:48:16 +0200 Subject: [PATCH 5/5] =?UTF-8?q?Fix=20#119:=20SMTP-/Nextcloud-Passw=C3=B6rt?= =?UTF-8?q?er=20verschl=C3=BCsselt=20gespeichert,=20kein=20Klartext=20im?= =?UTF-8?q?=20HTML=20mehr?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Changes.md | 9 +++ migrations/085_encrypt_settings_passwords.sql | 5 ++ src/Controllers/SettingsController.php | 7 ++ src/Models/Setting.php | 12 +++- src/Views/settings/form.php | 6 +- tests/52_SettingsPasswordEncryptionTest.php | 64 +++++++++++++++++++ 6 files changed, 97 insertions(+), 6 deletions(-) create mode 100644 migrations/085_encrypt_settings_passwords.sql create mode 100644 tests/52_SettingsPasswordEncryptionTest.php diff --git a/Changes.md b/Changes.md index d5a5ad5..ffbdce8 100644 --- a/Changes.md +++ b/Changes.md @@ -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). diff --git a/migrations/085_encrypt_settings_passwords.sql b/migrations/085_encrypt_settings_passwords.sql new file mode 100644 index 0000000..ef4166c --- /dev/null +++ b/migrations/085_encrypt_settings_passwords.sql @@ -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; diff --git a/src/Controllers/SettingsController.php b/src/Controllers/SettingsController.php index aec7af0..3c287e3 100644 --- a/src/Controllers/SettingsController.php +++ b/src/Controllers/SettingsController.php @@ -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)) : ''; diff --git a/src/Models/Setting.php b/src/Models/Setting.php index d724f24..0d38f74 100644 --- a/src/Models/Setting.php +++ b/src/Models/Setting.php @@ -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%', diff --git a/src/Views/settings/form.php b/src/Views/settings/form.php index e800398..1f93488 100644 --- a/src/Views/settings/form.php +++ b/src/Views/settings/form.php @@ -327,9 +327,8 @@

+ placeholder="">
Empfohlen: App-Passwort aus Nextcloud → Einstellungen → Sicherheit
@@ -381,7 +380,8 @@
- +
diff --git a/tests/52_SettingsPasswordEncryptionTest.php b/tests/52_SettingsPasswordEncryptionTest.php new file mode 100644 index 0000000..e1ec062 --- /dev/null +++ b/tests/52_SettingsPasswordEncryptionTest.php @@ -0,0 +1,64 @@ + $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(); + } + } +}