From: Aki Tuomi <aki.tuomi@open-xchange.com>
Date: Wed, 18 Nov 2020 07:43:07 +0000 (+0200)
Subject: lib-mail: message-decoder - Fix CTE parser to accept only syntaxically valid values
X-Git-Tag: 2.3.14.rc1~298
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=2e100e4531886c71d57c08af9453d7874ebe6739;p=thirdparty%2Fdovecot%2Fcore.git

lib-mail: message-decoder - Fix CTE parser to accept only syntaxically valid values
---

diff --git a/src/lib-mail/message-decoder.c b/src/lib-mail/message-decoder.c
index 2f4cf19269..845b3d5e3a 100644
--- a/src/lib-mail/message-decoder.c
+++ b/src/lib-mail/message-decoder.c
@@ -93,7 +93,17 @@ enum message_cte message_decoder_parse_cte(const struct message_header_line *hdr
 	rfc822_parser_init(&parser, hdr->full_value, hdr->full_value_len, NULL);
 
 	rfc822_skip_lwsp(&parser);
-	(void)rfc822_parse_mime_token(&parser, value);
+
+	/* Ensure we do not accidentically accept confused values like
+	   'base64 binary' or embedded NULs */
+	if (rfc822_parse_mime_token(&parser, value) == 1) {
+		rfc822_skip_lwsp(&parser);
+		/* RFC 2045 does not permit parameters for CTE,
+		   but in case someone uses them, we accept
+		   parameter separator ';' to be lenient. */
+		if (*parser.data != ';')
+			return MESSAGE_CTE_UNKNOWN;
+	}
 
 	message_cte = MESSAGE_CTE_UNKNOWN;
 	switch (str_len(value)) {
diff --git a/src/lib-mail/test-message-decoder.c b/src/lib-mail/test-message-decoder.c
index b33cec9734..aa331ec80c 100644
--- a/src/lib-mail/test-message-decoder.c
+++ b/src/lib-mail/test-message-decoder.c
@@ -316,6 +316,14 @@ static void test_message_decoder_invalid_content_transfer_encoding(void)
 "Content-Type: text/plain; charset=UTF-8\n\n"
 "Move \xE2\x99\x9A to \xE2\x99\x9B's \xE2\x99\x9D\n\n"
 "--1\n"
+"Content-Transfer-Encoding: 7-bit\n"
+"Content-Type: text/plain; charset=UTF-8\n\n"
+"Move \xE2\x99\x9A to \xE2\x99\x9B's \xE2\x99\x9D\n\n"
+"--1\n"
+"Content-Transfer-Encoding: 8-bit\n"
+"Content-Type: text/plain; charset=UTF-8\n\n"
+"Move \xE2\x99\x9A to \xE2\x99\x9B's \xE2\x99\x9D\n\n"
+"--1\n"
 "Content-Transfer-Encoding:\n"
 "Content-Type: text/plain; charset=UTF-8\n\n"
 "Move =E2=99=9A to =E2=99=9B's =E2=99=9D\n\n"
@@ -358,7 +366,7 @@ static void test_message_decoder_invalid_content_transfer_encoding(void)
 	test_assert(istream->stream_errno == 0);
 
 	part = parts;
-	test_assert(part->children_count == 4);
+	test_assert(part->children_count == 6);
 	part = part->children;
 	test_assert_strcmp(part->data->content_type, "text");
 	test_assert_strcmp(part->data->content_subtype, "plain");
@@ -372,10 +380,49 @@ static void test_message_decoder_invalid_content_transfer_encoding(void)
 	part = part->next;
 	test_assert(part->data->content_transfer_encoding == NULL);
 	part = part->next;
-	test_assert(part->data->content_transfer_encoding == NULL);
+	test_assert_strcmp(part->data->content_transfer_encoding, "7-bit");
+	part = part->next;
+	test_assert_strcmp(part->data->content_transfer_encoding, "8-bit");
+	part = part->next;
 	test_assert(part->next == NULL);
 	i_stream_unref(&istream);
 	pool_unref(&pool);
+
+#define X10(a) a a a a a a a a a a
+
+#undef TEST_CASE
+#define TEST_CASE(value, result) \
+	{ \
+		.hdr = { \
+			.name = "Content-Transfer-Encoding", \
+			.name_len = 25, \
+			.full_value = (const unsigned char*)value, \
+			.full_value_len = sizeof(value)-1, \
+		}, \
+		.cte = result, \
+	}
+
+	const struct {
+		const struct message_header_line hdr;
+		enum message_cte cte;
+	} test_case[] = {
+		TEST_CASE("(binary comment) base64", MESSAGE_CTE_BASE64),
+		TEST_CASE("(\"binary\" ( (comment) test) ) base64", MESSAGE_CTE_BASE64),
+		TEST_CASE("base64 binary", MESSAGE_CTE_UNKNOWN),
+		TEST_CASE("base64\0binary", MESSAGE_CTE_UNKNOWN),
+		TEST_CASE("\0binary", MESSAGE_CTE_UNKNOWN),
+		TEST_CASE("( " X10(X10(X10(X10("a")))) " ) base64", MESSAGE_CTE_BASE64),
+		TEST_CASE("( " X10(X10(X10(X10("a")))) " ) base64 ( " X10(X10(X10(X10("a")))) ")", MESSAGE_CTE_BASE64),
+		TEST_CASE("( base64", MESSAGE_CTE_UNKNOWN),
+		TEST_CASE("base64 (", MESSAGE_CTE_BASE64),
+		TEST_CASE(X10(X10(X10(X10(" ")))) " base64", MESSAGE_CTE_BASE64),
+		TEST_CASE("base64 ; logging-type=\"foobar\"", MESSAGE_CTE_BASE64),
+	};
+
+	for (size_t i = 0; i < N_ELEMENTS(test_case); i++) {
+		test_assert_idx(message_decoder_parse_cte(&test_case[i].hdr) == test_case[i].cte, i);
+	}
+
 	test_end();
 }