From 1ebe15c3a9807233cc92a92668b9090777daa9e1 Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Sun, 29 Jan 2017 18:03:40 +0000 Subject: [PATCH 1/1] CHUNKING: Reject messages with malformed line ending. Bug 2000 Actually test only the first header line, but still do full line-ending canonicalisation on the remainder of the message in case a Evil Person slips past that. --- doc/doc-docbook/spec.xfpt | 11 +-- src/src/functions.h | 2 + src/src/receive.c | 128 ++++++++++++++++++++++++----------- src/src/smtp_in.c | 2 +- test/log/0900 | 1 + test/rejectlog/0900 | 4 ++ test/scripts/0000-Basic/0900 | 26 +++++++ test/stdout/0900 | 33 +++++++++ 8 files changed, 162 insertions(+), 45 deletions(-) diff --git a/doc/doc-docbook/spec.xfpt b/doc/doc-docbook/spec.xfpt index 648078998..422edcee4 100644 --- a/doc/doc-docbook/spec.xfpt +++ b/doc/doc-docbook/spec.xfpt @@ -28934,7 +28934,11 @@ message body. Cutthrough delivery is not supported via transport-filters or when DKIM signing of outgoing messages is done, because it sends data to the ultimate destination before the entire message has been received from the source. -It is not supported for messages received with the SMTP PRDR option in use. +It is not supported for messages received with the SMTP PRDR +.new +or CHUNKING +.wen +options in use. Should the ultimate destination system positively accept or reject the mail, a corresponding indication is given to the source system and nothing is queued. @@ -37919,9 +37923,8 @@ lock will be lost at the instant of rename. .next .vindex "&$body_linecount$&" If you change the number of lines in the file, the value of -&$body_linecount$&, which is stored in the -H file, will be incorrect. At -present, this value is not used by Exim, but there is no guarantee that this -will always be the case. +&$body_linecount$&, which is stored in the -H file, will be incorrect and can +cause incomplete transmission of messages or undeliverable messages. .next If the message is in MIME format, you must take care not to break it. .next diff --git a/src/src/functions.h b/src/src/functions.h index 9c7dbe671..eeeb33390 100644 --- a/src/src/functions.h +++ b/src/src/functions.h @@ -102,6 +102,8 @@ extern int auth_xtextdecode(uschar *, uschar **); extern uschar *b64encode(uschar *, int); extern int b64decode(uschar *, uschar **); extern int bdat_getc(unsigned); +extern void bdat_flush_data(void); + extern void bits_clear(unsigned int *, size_t, int *); extern void bits_set(unsigned int *, size_t, int *); diff --git a/src/src/receive.c b/src/src/receive.c index 48cdff88c..6ce2823f5 100644 --- a/src/src/receive.c +++ b/src/src/receive.c @@ -898,8 +898,11 @@ return END_EOF; /* Variant of the above read_message_data_smtp() specialised for RFC 3030 -CHUNKING. We assume that the incoming has proper CRLF, so only have to scan -for and strip CR. On the downside there are more protocol reasons to stop. +CHUNKING. Accept input lines separated by either CRLF or CR or LF and write +LF-delimited spoolfile. Until we have wireformat spoolfiles, we need the +body_linecount accounting for proper re-expansion for the wire, so use +a cut-down version of the state-machine above; we don't need to do leading-dot +detection and unstuffing. Arguments: fout a FILE to which to write the message; NULL if skipping @@ -910,43 +913,73 @@ Returns: One of the END_xxx values indicating why it stopped reading static int read_message_bdat_smtp(FILE *fout) { -int ch; -int linelength = 0; +int ch_state = 0, linelength = 0, ch; -for (;;) switch (ch = bdat_getc(GETC_BUFFER_UNLIMITED)) +for(;;) { - case EOF: return END_EOF; - case EOD: return END_DOT; - case ERR: return END_PROTOCOL; + switch ((ch = (bdat_getc)(GETC_BUFFER_UNLIMITED))) + { + case EOF: return END_EOF; + case EOD: return END_DOT; /* normal exit */ + case ERR: return END_PROTOCOL; + case '\0': body_zerocount++; break; + } + switch (ch_state) + { + case 0: /* After LF or CRLF */ + ch_state = 1; + /* fall through to handle as normal uschar. */ - case '\r': - body_linecount++; - if (linelength > max_received_linelength) - max_received_linelength = linelength; - linelength = -1; - break; + case 1: /* Mid-line state */ + if (ch == '\n') + { + ch_state = 0; + body_linecount++; + if (linelength > max_received_linelength) + max_received_linelength = linelength; + linelength = -1; + } + else if (ch == '\r') + { + ch_state = 2; + continue; /* don't write CR */ + } + break; - case 0: - body_zerocount++; - /*FALLTHROUGH*/ - default: - message_size++; - linelength++; - if (fout) - { - if (fputc(ch, fout) == EOF) return END_WERROR; - if (message_size > thismessage_size_limit) return END_SIZE; - } -#ifdef notyet - if(ch == '\n') - (void) cutthrough_put_nl(); - else - { - uschar c = ch; - (void) cutthrough_puts(&c, 1); - } -#endif - break; + case 2: /* After (unwritten) CR */ + body_linecount++; + if (linelength > max_received_linelength) + max_received_linelength = linelength; + linelength = -1; + if (ch == '\n') + ch_state = 0; + else + { + message_size++; + if (fout != NULL && fputc('\n', fout) == EOF) return END_WERROR; + (void) cutthrough_put_nl(); + if (ch == '\r') continue; /* don't write CR */ + ch_state = 1; + } + break; + } + + /* Add the character to the spool file, unless skipping */ + + message_size++; + linelength++; + if (fout) + { + if (fputc(ch, fout) == EOF) return END_WERROR; + if (message_size > thismessage_size_limit) return END_SIZE; + } + if(ch == '\n') + (void) cutthrough_put_nl(); + else + { + uschar c = ch; + (void) cutthrough_puts(&c, 1); + } } /*NOTREACHED*/ } @@ -2084,6 +2117,21 @@ for (;;) } } + /* Reject CHUNKING messages that do not CRLF their first header line */ + + if (!first_line_ended_crlf && chunking_state > CHUNKING_OFFERED) + { + log_write(L_size_reject, LOG_MAIN|LOG_REJECT, "rejected from <%s>%s%s%s%s: " + "Non-CRLF-terminated header, under CHUNKING: message abandoned", + sender_address, + sender_fullhost ? " H=" : "", sender_fullhost ? sender_fullhost : US"", + sender_ident ? " U=" : "", sender_ident ? sender_ident : US""); + smtp_printf("552 Message header not CRLF terminated\r\n"); + bdat_flush_data(); + smtp_reply = US""; + goto TIDYUP; /* Skip to end of function */ + } + /* The line has been handled. If we have hit EOF, break out of the loop, indicating no pending data line. */ @@ -2108,7 +2156,7 @@ normal case). */ DEBUG(D_receive) { debug_printf(">>Headers received:\n"); - for (h = header_list->next; h != NULL; h = h->next) + for (h = header_list->next; h; h = h->next) debug_printf("%s", h->text); debug_printf("\n"); } @@ -2135,7 +2183,7 @@ if (filter_test != FTEST_NONE && header_list->next == NULL) /* Scan the headers to identify them. Some are merely marked for later processing; some are dealt with here. */ -for (h = header_list->next; h != NULL; h = h->next) +for (h = header_list->next; h; h = h->next) { BOOL is_resent = strncmpic(h->text, US"resent-", 7) == 0; if (is_resent) contains_resent_headers = TRUE; @@ -2351,7 +2399,7 @@ if (extract_recip) /* Now scan the headers */ - for (h = header_list->next; h != NULL; h = h->next) + for (h = header_list->next; h; h = h->next) { if ((h->type == htype_to || h->type == htype_cc || h->type == htype_bcc) && (!contains_resent_headers || strncmpic(h->text, US"resent-", 7) == 0)) @@ -2845,11 +2893,11 @@ We start at the second header, skipping our own Received:. This rewriting is documented as happening *after* recipient addresses are taken from the headers by the -t command line option. An added Sender: gets rewritten here. */ -for (h = header_list->next; h != NULL; h = h->next) +for (h = header_list->next; h; h = h->next) { header_line *newh = rewrite_header(h, NULL, NULL, global_rewrite_rules, rewrite_existflags, TRUE); - if (newh != NULL) h = newh; + if (newh) h = newh; } diff --git a/src/src/smtp_in.c b/src/src/smtp_in.c index 8f4309686..fa4bb99ed 100644 --- a/src/src/smtp_in.c +++ b/src/src/smtp_in.c @@ -600,7 +600,7 @@ next_cmd: } } -static void +void bdat_flush_data(void) { while (chunking_data_left > 0) diff --git a/test/log/0900 b/test/log/0900 index 8ce3bcb0f..cf02d983a 100644 --- a/test/log/0900 +++ b/test/log/0900 @@ -9,3 +9,4 @@ 1999-03-02 09:44:33 SMTP connection from (tester) [127.0.0.1] lost while reading message data 1999-03-02 09:44:33 10HmbB-0005vi-00 <= someone@some.domain H=(tester) [127.0.0.1] P=esmtp K S=sss for CALLER@test.ex 1999-03-02 09:44:33 H=(tester) [127.0.0.1] F= rejected RCPT : relay not permitted +1999-03-02 09:44:33 rejected from H=(tester) [127.0.0.1]: Non-CRLF-terminated header, under CHUNKING: message abandoned diff --git a/test/rejectlog/0900 b/test/rejectlog/0900 index 4c194b510..e5eb296d5 100644 --- a/test/rejectlog/0900 +++ b/test/rejectlog/0900 @@ -1,3 +1,7 @@ ******** SERVER ******** 1999-03-02 09:44:33 H=(tester) [127.0.0.1] F= rejected RCPT : relay not permitted +1999-03-02 09:44:33 rejected from H=(tester) [127.0.0.1]: Non-CRLF-terminated header, under CHUNKING: message abandoned +Envelope-from: +Envelope-to: + To: Susan@random.com diff --git a/test/scripts/0000-Basic/0900 b/test/scripts/0000-Basic/0900 index 9e014c43e..4503ae0c0 100644 --- a/test/scripts/0000-Basic/0900 +++ b/test/scripts/0000-Basic/0900 @@ -215,5 +215,31 @@ quit ??? 221 **** # +# plain, small message (no body) +# header line with bad line-ending +client 127.0.0.1 PORT_D +??? 220 +ehlo tester +??? 250- +??? 250-SIZE +??? 250-8BITMIME +??? 250-PIPELINING +??? 250-CHUNKING +??? 250 HELP +mail from:someone@some.domain +??? 250 +rcpt to:CALLER@test.ex +??? 250 +bdat 87 last +>>> To: Susan@random.com\n +From: Sam@random.com +Subject: This is a Bodyless test message + +??? 552 +quit +??? 221 +**** +# +# killdaemon no_msglog_check diff --git a/test/stdout/0900 b/test/stdout/0900 index 70c527c17..72269fad9 100644 --- a/test/stdout/0900 +++ b/test/stdout/0900 @@ -296,3 +296,36 @@ Connecting to 127.0.0.1 port 1225 ... connected ??? 221 <<< 221 testhost.test.ex closing connection End of script +Connecting to 127.0.0.1 port 1225 ... connected +??? 220 +<<< 220 testhost.test.ex ESMTP Exim x.yz Tue, 2 Mar 1999 09:44:33 +0000 +>>> ehlo tester +??? 250- +<<< 250-testhost.test.ex Hello tester [127.0.0.1] +??? 250-SIZE +<<< 250-SIZE 52428800 +??? 250-8BITMIME +<<< 250-8BITMIME +??? 250-PIPELINING +<<< 250-PIPELINING +??? 250-CHUNKING +<<< 250-CHUNKING +??? 250 HELP +<<< 250 HELP +>>> mail from:someone@some.domain +??? 250 +<<< 250 OK +>>> rcpt to:CALLER@test.ex +??? 250 +<<< 250 Accepted +>>> bdat 87 last +>>> To: Susan@random.com\n +>>> From: Sam@random.com +>>> Subject: This is a Bodyless test message +>>> +??? 552 +<<< 552 Message header not CRLF terminated +>>> quit +??? 221 +<<< 221 testhost.test.ex closing connection +End of script -- 2.30.2