@ZaphodB fix(security): strip line breaks from decoded and outgoing header values

DecodeHeaderValue removed CR and LF before decoding encoded-words, so an
encoded-word could still decode to a line break. A message whose Message-ID
was =?utf-8?b?...?= with CRLF inside reached the client with the break intact,
the client copies Message-ID into In-Reply-To when replying or forwarding, and
SetInReplyTo only trimmed it. The reply then carried header lines chosen by the
sender of the original, or a body if the payload held a blank line, sent and
DKIM-signed as the user and saved to Sent. Subject on Edit-as-new had the same
shape through the ASCII path.

Strip CR, LF and NUL in two places: after decoding in DecodeHeaderValue, and in
Header itself, which every outgoing header is built through, so a value that
arrives by another route (client parameters, plugins) cannot break the line
either. Folding is unaffected; __toString adds it back.

test/header-injection.php covers B and Q encoded-words, parsing, outgoing
In-Reply-To and Subject, and that long values still fold.

snappymail/v/0.0.0/app/libraries/MailSo/Base/Utils.php

@@ -300,7 +300,11 @@ abstract class Utils
 			$sValue = static::ConvertEncoding($sValue, $sMainCharset, Enumerations\Charset::UTF_8);
 		}
 
-		return $sValue;
+		// The strip above runs before decoding, so an encoded-word such as
+		// =?utf-8?b?...?= can still decode to a CR or LF. A decoded header value
+		// is one line; a line break here would be carried into the headers of a
+		// reply (In-Reply-To is copied from Message-ID) and inject new ones.
+		return \preg_replace('/[\r\n\0]+/', ' ', $sValue);
 	}
 
 	public static function RemoveHeaderFromHeaders(string $sIncHeaders, array $aHeadersToRemove = array()) : string

snappymail/v/0.0.0/app/libraries/MailSo/Mime/Header.php

@@ -37,8 +37,13 @@ class Header implements \JsonSerializable
 
 	private function initInputData(string $sName, string $sValue, string $sEncodedValueForReparse) : void
 	{
-		$this->sName = \trim($sName);
-		$this->sFullValue = \trim($sValue);
+		// A header is one logical line. A CR or LF in a name or value lets the
+		// value end its own header and start another (or the body), and values
+		// reach here from decoded encoded-words and from client parameters, so
+		// no caller can be relied on to have stripped them. Folding is added
+		// back by __toString().
+		$this->sName = \trim(\preg_replace('/[\r\n\0]+/', ' ', $sName));
+		$this->sFullValue = \trim(\preg_replace('/[\r\n\0]+/', ' ', $sValue));
 		$this->sEncodedValue = '';
 
 		if (\strlen($sEncodedValueForReparse) && ($this->IsEmail() || $this->IsSubject() || $this->IsParameterized())) {

test/header-injection.php

@@ -0,0 +1,52 @@
+<?php
+
+// Run with: php test/header-injection.php
+define('APP_VERSION', 'test');
+spl_autoload_register(static function (string $class): void {
+	$file = dirname(__DIR__).'/tachyon/v/0.0.0/app/libraries/'.str_replace('\\', '/', $class).'.php';
+	if (is_file($file)) {
+		require_once $file;
+	}
+});
+
+function check(bool $condition, string $message): void
+{
+	if (!$condition) {
+		throw new RuntimeException($message);
+	}
+}
+
+// An encoded-word that decodes to a line break must not survive decoding.
+$payload = "<x@example.com>\r\nBcc: attacker@example.net";
+$encoded = '=?utf-8?b?'.base64_encode($payload).'?=';
+$decoded = \MailSo\Base\Utils::DecodeHeaderValue($encoded);
+check(!preg_match('/[\r\n]/', $decoded), 'B-encoded CRLF survived DecodeHeaderValue');
+check(str_contains($decoded, 'Bcc: attacker@example.net'), 'decoded text was lost, not just its line break');
+
+$decoded = \MailSo\Base\Utils::DecodeHeaderValue('=?utf-8?q?a=0D=0AX-Injected:_1?=');
+check(!preg_match('/[\r\n]/', $decoded), 'Q-encoded CRLF survived DecodeHeaderValue');
+
+// The same through header parsing, as for an incoming Message-ID.
+$header = \MailSo\Mime\Header::NewInstanceFromEncodedString("Message-ID: {$encoded}");
+check(!preg_match('/[\r\n]/', $header->Value()), 'parsed Message-ID kept a line break');
+
+// Outgoing: a value handed straight to the message (In-Reply-To and Subject come
+// from client parameters) must serialise as one header, whatever it contains.
+$message = new \MailSo\Mime\Message();
+$message->SetInReplyTo("<x@example.com>\r\nBcc: attacker@example.net");
+$message->SetSubject("Re: hello\nX-Injected: 1\r\n\r\nbody text");
+$raw = stream_get_contents($message->ToStream());
+[$headers] = explode("\r\n\r\n", $raw, 2);
+foreach (explode("\r\n", $headers) as $line) {
+	check(!preg_match('/^(Bcc|X-Injected):/i', $line), "injected header line: {$line}");
+}
+check(!str_contains($headers, "\n\n"), 'blank line injected into the header block');
+check(str_contains($raw, 'In-Reply-To: <x@example.com> Bcc: attacker@example.net'), 'In-Reply-To was not kept as one line');
+
+// Folding must still work: a long value is wrapped with CRLF + space only.
+$header = new \MailSo\Mime\Header('Subject', str_repeat('word ', 40));
+foreach (array_slice(explode("\r\n", (string) $header), 1) as $continuation) {
+	check(str_starts_with($continuation, ' '), 'folded line does not start with whitespace');
+}
+
+echo "header-injection: ok\n";