Fix #148: SMTP-Header-/Kommando-Injection über mail_from_email behoben
This commit is contained in:
parent
b5c2c672c8
commit
acd32a76a5
4 changed files with 98 additions and 3 deletions
|
|
@ -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)
|
## 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).
|
- 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).
|
||||||
|
|
|
||||||
|
|
@ -85,7 +85,7 @@ class Setting
|
||||||
$data['mail_smtp_user'] ?: '',
|
$data['mail_smtp_user'] ?: '',
|
||||||
!empty($data['mail_smtp_pass']) ? Crypto::encrypt($data['mail_smtp_pass']) : '',
|
!empty($data['mail_smtp_pass']) ? Crypto::encrypt($data['mail_smtp_pass']) : '',
|
||||||
$data['mail_from_name'] ?: '',
|
$data['mail_from_name'] ?: '',
|
||||||
$data['mail_from_email'] ?: '',
|
self::sanitizeFromEmail($data['mail_from_email'] ?? ''),
|
||||||
$data['mail_subject_template'] ?: 'Rechnung %RNR%',
|
$data['mail_subject_template'] ?: 'Rechnung %RNR%',
|
||||||
$data['mail_body_template'] ?: null,
|
$data['mail_body_template'] ?: null,
|
||||||
max(50, (int)($data['logo_max_width'] ?? 600)),
|
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
|
public static function setLogo(string $path): void
|
||||||
{
|
{
|
||||||
Database::execute('UPDATE company_settings SET logo_path=? WHERE id=1', [$path]);
|
Database::execute('UPDATE company_settings SET logo_path=? WHERE id=1', [$path]);
|
||||||
|
|
|
||||||
|
|
@ -122,7 +122,7 @@ class SmtpMailer
|
||||||
?string $attachContent,
|
?string $attachContent,
|
||||||
?string $attachName,
|
?string $attachName,
|
||||||
): void {
|
): void {
|
||||||
$this->cmd('MAIL FROM:<' . $this->fromEmail . '>');
|
$this->cmd('MAIL FROM:<' . $this->sanitizeAddress($this->fromEmail) . '>');
|
||||||
$this->expect(250);
|
$this->expect(250);
|
||||||
|
|
||||||
foreach ($to as $addr) {
|
foreach ($to as $addr) {
|
||||||
|
|
@ -150,7 +150,7 @@ class SmtpMailer
|
||||||
?string $attachName,
|
?string $attachName,
|
||||||
): string {
|
): string {
|
||||||
$date = date('r');
|
$date = date('r');
|
||||||
$fromHdr = $this->encodeHeader($this->fromName) . ' <' . $this->fromEmail . '>';
|
$fromHdr = $this->encodeHeader($this->fromName) . ' <' . $this->sanitizeAddress($this->fromEmail) . '>';
|
||||||
$toHdr = implode(', ', $to);
|
$toHdr = implode(', ', $to);
|
||||||
$subjHdr = $this->encodeHeader($subject);
|
$subjHdr = $this->encodeHeader($subject);
|
||||||
$msgId = '<' . uniqid('sk', true) . '@' . (gethostname() ?: 'localhost') . '>';
|
$msgId = '<' . uniqid('sk', true) . '@' . (gethostname() ?: 'localhost') . '>';
|
||||||
|
|
@ -228,6 +228,12 @@ class SmtpMailer
|
||||||
} while ($line[3] === '-');
|
} 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
|
private function encodeHeader(string $value): string
|
||||||
{
|
{
|
||||||
$value = str_replace(["\r", "\n"], '', $value);
|
$value = str_replace(["\r", "\n"], '', $value);
|
||||||
|
|
|
||||||
72
tests/69_MailFromEmailInjectionTest.php
Normal file
72
tests/69_MailFromEmailInjectionTest.php
Normal file
|
|
@ -0,0 +1,72 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
declare(strict_types=1);
|
||||||
|
|
||||||
|
use Steuerkram\Models\Setting;
|
||||||
|
use Steuerkram\Services\SmtpMailer;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Test 69: SMTP-Header-/Kommando-Injection über mail_from_email verhindert (Fix #148).
|
||||||
|
*/
|
||||||
|
class MailFromEmailInjectionTest extends TestCase
|
||||||
|
{
|
||||||
|
protected string $testName = 'Test 69: mail_from_email – Validierung & SmtpMailer-Härtung';
|
||||||
|
|
||||||
|
public function run(): void
|
||||||
|
{
|
||||||
|
$before = Setting::get();
|
||||||
|
$base = array_merge($before, [
|
||||||
|
'firma' => $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:<angreifer@evil.example>\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:<angreifer@evil.example>");
|
||||||
|
$this->assert(
|
||||||
|
!str_contains($result, "\r") && !str_contains($result, "\n"),
|
||||||
|
'sanitizeAddress() entfernt CR/LF aus der Absenderadresse'
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
Loading…
Add table
Reference in a new issue