Fix #126: E-Mail-Header-Injection über mail_subject_template verhindert

Changes.md

@@ -2,6 +2,13 @@
 
 ---
 
+## 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).
+- **`src/Models/Customer.php`:** `create()`/`update()` entfernen `\r`/`\n` aus `mail_subject_template` jetzt zentral über die neue Methode `sanitizeSubjectTemplate()`, bevor der Wert in der Datenbank landet.
+- **`src/Services/SmtpMailer.php`:** `encodeHeader()` entfernt zusätzlich (Defense-in-Depth) `\r`/`\n` aus jedem Header-Wert (Subject, From-Name, Anhang-Dateiname), bevor dieser in die rohe SMTP-DATA eingebettet wird – wirksam unabhängig davon, woher der Wert stammt.
+- **`tests/55_MailSubjectHeaderInjectionTest.php`:** Neuer Test prüft, dass CRLF-Payloads in der Betreff-Vorlage beim Speichern entfernt werden und dass `SmtpMailer::encodeHeader()` CR/LF auch bei direkter Übergabe entfernt.
+
 ## 2026-07-10 – Fix: `update.sh` meldet Testfehler per Textsuche statt Exit-Code (Issue #124)
 
 - **Niedrig:** `scripts/update.sh` prüfte den Erfolg der Test-Suite nicht über den Exit-Code von `tests/run.php`, sondern per `grep` nach den Wörtern „FEHLER“, „FAILED“, „fehlgeschlagen“ bzw. „failed“ im Test-Output. Diese Wörter kommen jedoch auch in Testdaten und erwarteten Fehlermeldungen bestandener Tests vor (z.B. Test 47 EventLog, Test 50 SMTP), sodass trotz „✓ Alle Tests bestanden“ eine falsche Warnung ausgegeben wurde. Zusätzlich war `TEST_EXIT=${PIPESTATUS[0]:-0}` wirkungslos, da `PIPESTATUS` sich nicht auf die vorangegangene Command-Substitution bezieht.

src/Models/Customer.php

@@ -59,7 +59,7 @@ class Customer
                 $data['steuernummer'] ?? null,
                 (int)($data['payment_days'] ?? 14),
                 $data['notes'] ?? null,
-                ($data['mail_subject_template'] ?? '') !== '' ? trim($data['mail_subject_template']) : null,
+                self::sanitizeSubjectTemplate($data['mail_subject_template'] ?? ''),
                 ($data['mail_body_template']    ?? '') !== '' ? trim($data['mail_body_template'])    : null,
             ]
         );
@@ -90,13 +90,24 @@ class Customer
                 $data['steuernummer'] ?? null,
                 (int)($data['payment_days'] ?? 14),
                 $data['notes'] ?? null,
-                ($data['mail_subject_template'] ?? '') !== '' ? trim($data['mail_subject_template']) : null,
+                self::sanitizeSubjectTemplate($data['mail_subject_template'] ?? ''),
                 ($data['mail_body_template']    ?? '') !== '' ? trim($data['mail_body_template'])    : null,
                 $id,
             ]
         );
     }
 
+    /**
+     * Entfernt eingebettete CRLF/LF aus der Betreff-Vorlage, damit sie beim
+     * Rechnungsversand nicht als zusätzlicher SMTP-Header (z.B. Bcc:) injiziert
+     * werden kann (CWE-93).
+     */
+    private static function sanitizeSubjectTemplate(string $value): ?string
+    {
+        $value = trim(str_replace(["\r", "\n"], ' ', $value));
+        return $value !== '' ? $value : null;
+    }
+
     public static function archive(int $id): void
     {
         Database::execute('UPDATE customers SET archived=1, updated_at=NOW() WHERE id=?', [$id]);

src/Services/SmtpMailer.php

@@ -230,6 +230,8 @@ class SmtpMailer
 
     private function encodeHeader(string $value): string
     {
+        $value = str_replace(["\r", "\n"], '', $value);
+
         if (mb_detect_encoding($value, 'ASCII', true)) {
             return $value;
         }

tests/55_MailSubjectHeaderInjectionTest.php

@@ -0,0 +1,67 @@
+<?php
+
+declare(strict_types=1);
+
+use Steuerkram\Models\Customer;
+use Steuerkram\Services\SmtpMailer;
+
+class MailSubjectHeaderInjectionTest extends TestCase
+{
+    protected string $testName = 'Test 55: Header-Injection über mail_subject_template verhindert';
+
+    public function run(): void
+    {
+        // CRLF-Payload in der Betreff-Vorlage wird beim Speichern entfernt
+        $id = Customer::create([
+            'nachname'              => 'Header-Injection-Test',
+            'strasse'               => 'Musterweg 1',
+            'plz'                   => '12345',
+            'ort'                   => 'Musterstadt',
+            'mail_subject_template' => "Rechnung %RNR%\r\nBcc: angreifer@example.com",
+        ]);
+        $this->assertGreaterThan(0, $id, 'Kunde angelegt');
+
+        $customer = Customer::find($id);
+        $this->assert(
+            !str_contains($customer['mail_subject_template'], "\r") && !str_contains($customer['mail_subject_template'], "\n"),
+            'Gespeicherte Betreff-Vorlage enthält kein CR/LF mehr'
+        );
+        $this->assert(
+            str_contains($customer['mail_subject_template'], 'Bcc: angreifer@example.com'),
+            'Injizierter Text bleibt als Klartext im Betreff, wird aber nicht mehr als Header interpretierbar'
+        );
+
+        // Update mit LF-only-Payload wird ebenfalls bereinigt
+        Customer::update($id, [
+            'nachname'              => 'Header-Injection-Test',
+            'strasse'               => 'Musterweg 1',
+            'plz'                   => '12345',
+            'ort'                   => 'Musterstadt',
+            'mail_subject_template' => "Rechnung %RNR%\nBcc: angreifer2@example.com",
+        ]);
+        $customer = Customer::find($id);
+        $this->assert(
+            !str_contains($customer['mail_subject_template'], "\n"),
+            'Betreff-Vorlage nach Update ebenfalls ohne LF gespeichert'
+        );
+
+        // Defense-in-depth: SmtpMailer::encodeHeader() entfernt CRLF selbst, falls ein
+        // CRLF-Betreff trotzdem irgendwo in den Mailer gelangt (Reflection auf private Methode)
+        $mailer = new SmtpMailer(
+            host:       '127.0.0.1',
+            port:       1,
+            encryption: 'none',
+            user:       '',
+            pass:       '',
+            fromEmail:  'absender@example.com',
+            fromName:   'Absender',
+        );
+        $ref    = new \ReflectionMethod($mailer, 'encodeHeader');
+        $ref->setAccessible(true);
+        $result = $ref->invoke($mailer, "Rechnung 2026-1\r\nBcc: angreifer@example.com");
+        $this->assert(
+            !str_contains($result, "\r") && !str_contains($result, "\n"),
+            'encodeHeader() entfernt CR/LF aus Header-Werten'
+        );
+    }
+}