From acd32a76a56a09ae03b85812fb099902ea328328 Mon Sep 17 00:00:00 2001 From: fds2610-bot Date: Thu, 30 Jul 2026 17:57:44 +0200 Subject: [PATCH] =?UTF-8?q?Fix=20#148:=20SMTP-Header-/Kommando-Injection?= =?UTF-8?q?=20=C3=BCber=20mail=5Ffrom=5Femail=20behoben?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Changes.md | 7 +++ src/Models/Setting.php | 12 ++++- src/Services/SmtpMailer.php | 10 +++- tests/69_MailFromEmailInjectionTest.php | 72 +++++++++++++++++++++++++ 4 files changed, 98 insertions(+), 3 deletions(-) create mode 100644 tests/69_MailFromEmailInjectionTest.php diff --git a/Changes.md b/Changes.md index eb8e69f..5bd7a48 100644 --- a/Changes.md +++ b/Changes.md @@ -2,6 +2,13 @@ --- +## 2026-07-30 – Fix: SMTP-Header-/Kommando-Injection über mail_from_email (Issue #148) + +- `SmtpMailer::sendMessage()`/`buildMessage()` verwendeten `fromEmail` roh sowohl im `MAIL FROM:<...>`-SMTP-Kommando als auch im `From:`-Header, während alle anderen dynamischen Werte (Empfänger, `fromName`, `subject`, Anhangsname) bereits validiert bzw. CRLF-bereinigt wurden. `mail_from_email` (Firmeneinstellung, per POST setzbar) durchlief keine serverseitige Prüfung – ein eingeloggter Nutzer konnte per CRLF-Payload zusätzliche SMTP-Kommandos (z.B. eine verdeckte `RCPT TO`-Zeile) einschleusen oder den `From:`-Header manipulieren (CWE-93). +- **`src/Models/Setting.php`:** `Setting::update()` validiert `mail_from_email` jetzt über `filter_var(..., FILTER_VALIDATE_EMAIL)` (neue private Methode `sanitizeFromEmail()`); ungültige Werte (inkl. CRLF-Payloads) werden verworfen statt gespeichert. +- **`src/Services/SmtpMailer.php`:** Defense-in-Depth – neue Methode `sanitizeAddress()` entfernt CR/LF aus `fromEmail`, bevor dieser im `MAIL FROM:`-Kommando und im `From:`-Header verwendet wird. +- **`tests/69_MailFromEmailInjectionTest.php`:** Neuer Test prüft, dass ungültige/CRLF-behaftete Absenderadressen beim Speichern verworfen werden und dass `SmtpMailer::sanitizeAddress()` CR/LF zuverlässig entfernt. + ## 2026-07-27 – Fix: Produktions-Härtung der PHP-Fehlerausgabe (Issue #142) - Das Docker-Image (`php:8.3-apache`) übernahm die kompilierten PHP-Defaults, bei denen `display_errors` aktiviert ist – im offiziellen Image wird `php.ini-production` nicht automatisch aktiviert. In Kombination mit `PDO::ATTR_ERRMODE_EXCEPTION` (`src/Database.php`) und fehlendem globalem Error-Handler hätte jeder unerwartete Fehler (SQL-Fehler, DB-Verbindungsproblem, Typfehler) eine vollständige PHP-Fehlermeldung inkl. Stacktrace, absoluten Serverpfaden und ggf. SQL-Details direkt an den Client ausgegeben (CWE-209, Information Exposure Through an Error Message) – auch für nicht angemeldete Nutzer (`/login` ist vor `Auth::require()` erreichbar). diff --git a/src/Models/Setting.php b/src/Models/Setting.php index 0d38f74..7f65e95 100644 --- a/src/Models/Setting.php +++ b/src/Models/Setting.php @@ -85,7 +85,7 @@ class Setting $data['mail_smtp_user'] ?: '', !empty($data['mail_smtp_pass']) ? Crypto::encrypt($data['mail_smtp_pass']) : '', $data['mail_from_name'] ?: '', - $data['mail_from_email'] ?: '', + self::sanitizeFromEmail($data['mail_from_email'] ?? ''), $data['mail_subject_template'] ?: 'Rechnung %RNR%', $data['mail_body_template'] ?: null, max(50, (int)($data['logo_max_width'] ?? 600)), @@ -99,6 +99,16 @@ class Setting ); } + /** + * Validiert die Absender-Adresse; bei ungültiger Eingabe wird der Wert verworfen, + * damit kein CRLF-Payload als SMTP-Kommando/Header injiziert werden kann (CWE-93). + */ + private static function sanitizeFromEmail(string $value): string + { + $value = trim($value); + return filter_var($value, FILTER_VALIDATE_EMAIL) !== false ? $value : ''; + } + public static function setLogo(string $path): void { Database::execute('UPDATE company_settings SET logo_path=? WHERE id=1', [$path]); diff --git a/src/Services/SmtpMailer.php b/src/Services/SmtpMailer.php index 0435f6b..8709cab 100644 --- a/src/Services/SmtpMailer.php +++ b/src/Services/SmtpMailer.php @@ -122,7 +122,7 @@ class SmtpMailer ?string $attachContent, ?string $attachName, ): void { - $this->cmd('MAIL FROM:<' . $this->fromEmail . '>'); + $this->cmd('MAIL FROM:<' . $this->sanitizeAddress($this->fromEmail) . '>'); $this->expect(250); foreach ($to as $addr) { @@ -150,7 +150,7 @@ class SmtpMailer ?string $attachName, ): string { $date = date('r'); - $fromHdr = $this->encodeHeader($this->fromName) . ' <' . $this->fromEmail . '>'; + $fromHdr = $this->encodeHeader($this->fromName) . ' <' . $this->sanitizeAddress($this->fromEmail) . '>'; $toHdr = implode(', ', $to); $subjHdr = $this->encodeHeader($subject); $msgId = '<' . uniqid('sk', true) . '@' . (gethostname() ?: 'localhost') . '>'; @@ -228,6 +228,12 @@ class SmtpMailer } while ($line[3] === '-'); } + /** Entfernt eingebettete CRLF, damit die Adresse nicht als SMTP-Kommando/Header-Injection missbraucht werden kann. */ + private function sanitizeAddress(string $value): string + { + return str_replace(["\r", "\n"], '', $value); + } + private function encodeHeader(string $value): string { $value = str_replace(["\r", "\n"], '', $value); diff --git a/tests/69_MailFromEmailInjectionTest.php b/tests/69_MailFromEmailInjectionTest.php new file mode 100644 index 0000000..8fbc904 --- /dev/null +++ b/tests/69_MailFromEmailInjectionTest.php @@ -0,0 +1,72 @@ + $before['firma'] ?: 'Testfirma', + 'strasse' => $before['strasse'] ?: 'Teststr. 1', + 'plz' => $before['plz'] ?: '12345', + 'ort' => $before['ort'] ?: 'Testort', + ]); + + // Gültige Adresse wird unverändert übernommen + Setting::update(array_merge($base, [ + 'mail_from_email' => 'rechnung@firma.example', + ])); + $this->assertEquals('rechnung@firma.example', Setting::get()['mail_from_email'], 'Gültige Absenderadresse wird gespeichert'); + + // CRLF-Payload (SMTP-Kommando-Injection) wird beim Speichern verworfen, nicht nur bereinigt + Setting::update(array_merge($base, [ + 'mail_from_email' => "admin@firma.example>\r\nRCPT TO:\r\nDATA", + ])); + $stored = Setting::get()['mail_from_email']; + $this->assert( + !str_contains($stored, "\r") && !str_contains($stored, "\n"), + 'Gespeicherte Absenderadresse enthält kein CR/LF' + ); + $this->assertEquals('', $stored, 'Ungültige (CRLF-behaftete) Absenderadresse wird verworfen statt gespeichert'); + + // Andere ungültige Werte (kein E-Mail-Format) werden ebenfalls verworfen + Setting::update(array_merge($base, [ + 'mail_from_email' => 'keine-email-adresse', + ])); + $this->assertEquals('', Setting::get()['mail_from_email'], 'Nicht-E-Mail-Wert wird verworfen'); + + // Ursprünglichen Zustand wiederherstellen + Setting::update(array_merge($base, [ + 'mail_from_email' => $before['mail_from_email'] ?? '', + ])); + + // Defense-in-depth: SmtpMailer::sanitizeAddress() entfernt CRLF selbst, + // falls eine mit CRLF behaftete Adresse trotzdem in den Mailer gelangt. + $mailer = new SmtpMailer( + host: '127.0.0.1', + port: 1, + encryption: 'none', + user: '', + pass: '', + fromEmail: 'absender@example.com', + fromName: 'Absender', + ); + $ref = new \ReflectionMethod($mailer, 'sanitizeAddress'); + $ref->setAccessible(true); + $result = $ref->invoke($mailer, "admin@firma.example>\r\nRCPT TO:"); + $this->assert( + !str_contains($result, "\r") && !str_contains($result, "\n"), + 'sanitizeAddress() entfernt CR/LF aus der Absenderadresse' + ); + } +}