diff --git a/Changes.md b/Changes.md index 6da9de4..3713275 100644 --- a/Changes.md +++ b/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. diff --git a/src/Models/Customer.php b/src/Models/Customer.php index 4814c16..2228617 100644 --- a/src/Models/Customer.php +++ b/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]); diff --git a/src/Services/SmtpMailer.php b/src/Services/SmtpMailer.php index b54b493..0435f6b 100644 --- a/src/Services/SmtpMailer.php +++ b/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; } diff --git a/tests/55_MailSubjectHeaderInjectionTest.php b/tests/55_MailSubjectHeaderInjectionTest.php new file mode 100644 index 0000000..7dafea1 --- /dev/null +++ b/tests/55_MailSubjectHeaderInjectionTest.php @@ -0,0 +1,67 @@ + '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' + ); + } +}