]> git.ipfire.org Git - thirdparty/dovecot/core.git/commitdiff
lib-mail: message-decoder - Fix CTE parser to accept only syntaxically valid values
authorAki Tuomi <aki.tuomi@open-xchange.com>
Wed, 18 Nov 2020 07:43:07 +0000 (09:43 +0200)
committertimo.sirainen <timo.sirainen@open-xchange.com>
Tue, 1 Dec 2020 14:33:32 +0000 (14:33 +0000)
src/lib-mail/message-decoder.c
src/lib-mail/test-message-decoder.c

index 2f4cf1926904ff03ba5c9a14057c5873803f9144..845b3d5e3aee805275bad9be1f3e63d3e9f64c71 100644 (file)
@@ -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)) {
index b33cec9734a4350bd73b943f05a8d0437f379c6b..aa331ec80cb0680467576e6c4711b34fc32bdcc2 100644 (file)
@@ -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();
 }