From 7a12331c6360968b141a0888e0bf04dd24145f23 Mon Sep 17 00:00:00 2001 From: Timo Sirainen Date: Fri, 22 Apr 2016 18:15:44 +0300 Subject: [PATCH] lib-mail: Fixed handling duplicate boundary prefixes. If inner MIME part had the same --boundary prefix as its parent(s) and the MIME part body started with the inner --boundary prefix, we didn't yet have it in the list of valid boundaries, so we thought that the outer boundary was found and the MIME headers were truncated. But due to an extra bug we still treated it as if it were the inner boundary, except the MIME part sizes/offsets were set wrong. This for example fixes a situation where FETCH [1.2.MIME] returns an extra newline before the actual headers. --- src/lib-mail/message-parser.c | 45 +++++++++-- src/lib-mail/test-message-parser.c | 116 +++++++++++++++++++++++++++++ 2 files changed, 154 insertions(+), 7 deletions(-) diff --git a/src/lib-mail/message-parser.c b/src/lib-mail/message-parser.c index a8177f3785..e936005d56 100644 --- a/src/lib-mail/message-parser.c +++ b/src/lib-mail/message-parser.c @@ -45,6 +45,7 @@ struct message_parser_ctx { struct message_block *block_r); unsigned int part_seen_content_type:1; + unsigned int multipart:1; unsigned int eof:1; }; @@ -504,6 +505,21 @@ static void parse_content_type(struct message_parser_ctx *ctx, } } +static bool block_is_at_eoh(const struct message_block *block) +{ + if (block->size < 1) + return FALSE; + if (block->data[0] == '\n') + return TRUE; + if (block->data[0] == '\r') { + if (block->size < 2) + return FALSE; + if (block->data[1] == '\n') + return TRUE; + } + return FALSE; +} + #define MUTEX_FLAGS \ (MESSAGE_PART_FLAG_MESSAGE_RFC822 | MESSAGE_PART_FLAG_MULTIPART) @@ -519,12 +535,30 @@ static int parse_next_header(struct message_parser_ctx *ctx, if ((ret = message_parser_read_more(ctx, block_r, &full)) == 0) return ret; + if (ret > 0 && block_is_at_eoh(block_r) && + ctx->last_boundary != NULL && + (part->flags & MESSAGE_PART_FLAG_IS_MIME) != 0) { + /* we are at the end of headers and we've determined that we're + going to start a multipart. add the boundary already here + at this point so we can reliably determine whether the + "\n--boundary" belongs to us or to a previous boundary. + this is a problem if the boundary prefixes are identical, + because MIME requires only the prefix to match. */ + parse_next_body_multipart_init(ctx); + ctx->multipart = TRUE; + } + /* before parsing the header see if we can find a --boundary from here. we're guaranteed to be at the beginning of the line here. */ if (ret > 0) { ret = ctx->boundaries == NULL ? -1 : boundary_line_find(ctx, block_r->data, block_r->size, full, &boundary); + if (ret > 0 && boundary->part == ctx->part) { + /* our own body begins with our own --boundary. + we don't want to handle that yet. */ + ret = -1; + } } if (ret < 0) { /* no boundary */ @@ -581,14 +615,10 @@ static int parse_next_header(struct message_parser_ctx *ctx, } /* end of headers */ - if ((part->flags & MESSAGE_PART_FLAG_MULTIPART) != 0 && - ctx->last_boundary == NULL) { - /* multipart type but no message boundary */ - part->flags = 0; - } if ((part->flags & MESSAGE_PART_FLAG_IS_MIME) == 0) { /* It's not MIME. Reset everything we found from Content-Type. */ + i_assert(!ctx->multipart); part->flags = 0; ctx->last_boundary = NULL; } @@ -615,8 +645,9 @@ static int parse_next_header(struct message_parser_ctx *ctx, i_assert((part->flags & MUTEX_FLAGS) != MUTEX_FLAGS); ctx->last_chr = '\n'; - if (ctx->last_boundary != NULL) { - parse_next_body_multipart_init(ctx); + if (ctx->multipart) { + i_assert(ctx->last_boundary == NULL); + ctx->multipart = FALSE; ctx->parse_next_block = parse_next_body_to_boundary; } else if (part->flags & MESSAGE_PART_FLAG_MESSAGE_RFC822) ctx->parse_next_block = parse_next_body_message_rfc822_init; diff --git a/src/lib-mail/test-message-parser.c b/src/lib-mail/test-message-parser.c index da56fb0568..7f7e7b060f 100644 --- a/src/lib-mail/test-message-parser.c +++ b/src/lib-mail/test-message-parser.c @@ -196,6 +196,120 @@ static const char input_msg[] = test_end(); } +static void test_message_parser_duplicate_mime_boundary(void) +{ +static const char input_msg[] = +"Content-Type: multipart/mixed; boundary=\"a\"\n" +"\n" +"--a\n" +"Content-Type: multipart/mixed; boundary=\"a\"\n" +"\n" +"--a\n" +"Content-Type: text/plain\n" +"\n" +"body\n"; + struct message_parser_ctx *parser; + struct istream *input; + struct message_part *parts; + struct message_block block; + pool_t pool; + int ret; + + test_begin("message parser duplicate mime boundary"); + pool = pool_alloconly_create("message parser", 10240); + input = test_istream_create(input_msg); + + parser = message_parser_init(pool, input, 0, 0); + while ((ret = message_parser_parse_next_block(parser, &block)) > 0) ; + test_assert(ret < 0); + test_assert(message_parser_deinit(&parser, &parts) == 0); + + test_assert(parts->flags == (MESSAGE_PART_FLAG_MULTIPART | MESSAGE_PART_FLAG_IS_MIME)); + test_assert(parts->header_size.lines == 2); + test_assert(parts->header_size.physical_size == 45); + test_assert(parts->header_size.virtual_size == 45+2); + test_assert(parts->body_size.lines == 7); + test_assert(parts->body_size.physical_size == 84); + test_assert(parts->body_size.virtual_size == 84+7); + test_assert(parts->children->flags == (MESSAGE_PART_FLAG_MULTIPART | MESSAGE_PART_FLAG_IS_MIME)); + test_assert(parts->children->physical_pos == 49); + test_assert(parts->children->header_size.lines == 2); + test_assert(parts->children->header_size.physical_size == 45); + test_assert(parts->children->header_size.virtual_size == 45+2); + test_assert(parts->children->body_size.lines == 4); + test_assert(parts->children->body_size.physical_size == 35); + test_assert(parts->children->body_size.virtual_size == 35+4); + test_assert(parts->children->children->flags == (MESSAGE_PART_FLAG_TEXT | MESSAGE_PART_FLAG_IS_MIME)); + test_assert(parts->children->children->physical_pos == 98); + test_assert(parts->children->children->header_size.lines == 2); + test_assert(parts->children->children->header_size.physical_size == 26); + test_assert(parts->children->children->header_size.virtual_size == 26+2); + test_assert(parts->children->children->body_size.lines == 1); + test_assert(parts->children->children->body_size.physical_size == 5); + test_assert(parts->children->children->body_size.virtual_size == 5+1); + + i_stream_unref(&input); + pool_unref(&pool); + test_end(); +} + +static void test_message_parser_continuing_mime_boundary(void) +{ +static const char input_msg[] = +"Content-Type: multipart/mixed; boundary=\"a\"\n" +"\n" +"--a\n" +"Content-Type: multipart/mixed; boundary=\"ab\"\n" +"\n" +"--ab\n" +"Content-Type: text/plain\n" +"\n" +"body\n"; + struct message_parser_ctx *parser; + struct istream *input; + struct message_part *parts; + struct message_block block; + pool_t pool; + int ret; + + test_begin("message parser continuing mime boundary"); + pool = pool_alloconly_create("message parser", 10240); + input = test_istream_create(input_msg); + + parser = message_parser_init(pool, input, 0, 0); + while ((ret = message_parser_parse_next_block(parser, &block)) > 0) ; + test_assert(ret < 0); + test_assert(message_parser_deinit(&parser, &parts) == 0); + + test_assert(parts->flags == (MESSAGE_PART_FLAG_MULTIPART | MESSAGE_PART_FLAG_IS_MIME)); + test_assert(parts->header_size.lines == 2); + test_assert(parts->header_size.physical_size == 45); + test_assert(parts->header_size.virtual_size == 45+2); + test_assert(parts->body_size.lines == 7); + test_assert(parts->body_size.physical_size == 86); + test_assert(parts->body_size.virtual_size == 86+7); + test_assert(parts->children->flags == (MESSAGE_PART_FLAG_MULTIPART | MESSAGE_PART_FLAG_IS_MIME)); + test_assert(parts->children->physical_pos == 49); + test_assert(parts->children->header_size.lines == 2); + test_assert(parts->children->header_size.physical_size == 46); + test_assert(parts->children->header_size.virtual_size == 46+2); + test_assert(parts->children->body_size.lines == 4); + test_assert(parts->children->body_size.physical_size == 36); + test_assert(parts->children->body_size.virtual_size == 36+4); + test_assert(parts->children->children->flags == (MESSAGE_PART_FLAG_TEXT | MESSAGE_PART_FLAG_IS_MIME)); + test_assert(parts->children->children->physical_pos == 100); + test_assert(parts->children->children->header_size.lines == 2); + test_assert(parts->children->children->header_size.physical_size == 26); + test_assert(parts->children->children->header_size.virtual_size == 26+2); + test_assert(parts->children->children->body_size.lines == 1); + test_assert(parts->children->children->body_size.physical_size == 5); + test_assert(parts->children->children->body_size.virtual_size == 5+1); + + i_stream_unref(&input); + pool_unref(&pool); + test_end(); +} + static void test_message_parser_no_eoh(void) { static const char input_msg[] = "a:b\n"; @@ -228,6 +342,8 @@ int main(void) static void (*test_functions[])(void) = { test_message_parser_small_blocks, test_message_parser_truncated_mime_headers, + test_message_parser_duplicate_mime_boundary, + test_message_parser_continuing_mime_boundary, test_message_parser_no_eoh, NULL }; -- 2.47.3