diff --git a/snappymail/v/0.0.0/app/libraries/MailSo/Base/Utils.php b/snappymail/v/0.0.0/app/libraries/MailSo/Base/Utils.php index f16598be8..d96e2f74c 100644 --- a/snappymail/v/0.0.0/app/libraries/MailSo/Base/Utils.php +++ b/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 diff --git a/snappymail/v/0.0.0/app/libraries/MailSo/Mime/Header.php b/snappymail/v/0.0.0/app/libraries/MailSo/Mime/Header.php index f48cdf0f4..1abbb989e 100644 --- a/snappymail/v/0.0.0/app/libraries/MailSo/Mime/Header.php +++ b/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())) { diff --git a/test/header-injection.php b/test/header-injection.php new file mode 100644 index 000000000..a1115151d --- /dev/null +++ b/test/header-injection.php @@ -0,0 +1,52 @@ +\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("\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: 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";