From 315206fbf2d75c73de73deab89443ab645d96525 Mon Sep 17 00:00:00 2001 From: Jeremy Harris Date: Thu, 26 May 2022 20:11:43 +0100 Subject: [PATCH] CHUNKING: handle protocol errors during reception --- src/src/receive.c | 70 ++++++++++++++++++++++-------------- src/src/smtp_in.c | 7 ++-- test/log/0900 | 3 +- test/scripts/0000-Basic/0900 | 27 +++++++++++++- test/stdout/0900 | 42 +++++++++++++++++++++- 5 files changed, 117 insertions(+), 32 deletions(-) diff --git a/src/src/receive.c b/src/src/receive.c index a6ecb0a90..0a27c7950 100644 --- a/src/src/receive.c +++ b/src/src/receive.c @@ -967,7 +967,7 @@ Returns: One of the END_xxx values indicating why it stopped reading */ static int -read_message_bdat_smtp(FILE *fout) +read_message_bdat_smtp(FILE * fout) { int linelength = 0, ch; enum CH_STATE ch_state = LF_SEEN; @@ -1073,7 +1073,7 @@ for(;;) } static int -read_message_bdat_smtp_wire(FILE *fout) +read_message_bdat_smtp_wire(FILE * fout) { int ch; @@ -1889,12 +1889,15 @@ for (;;) /* If we hit EOF on a SMTP connection, it's an error, since incoming SMTP must have a correct "." terminator. */ - if (ch == EOF && smtp_input /* && !smtp_batched_input */) - { - smtp_reply = handle_lost_connection(US" (header)"); - smtp_yield = FALSE; - goto TIDYUP; /* Skip to end of function */ - } + if (smtp_input /* && !smtp_batched_input */) + if (ch == EOF) + { + smtp_reply = handle_lost_connection(US" (header)"); + smtp_yield = FALSE; + goto TIDYUP; /* Skip to end of function */ + } + else if (ch == ERR) + goto TIDYUP; /* See if we are at the current header's size limit - there must be at least four bytes left. This allows for the new character plus a zero, plus two for @@ -1934,7 +1937,7 @@ for (;;) those from data files use just LF. Treat LF in local SMTP input as a terminator too. Treat EOF as a line terminator always. */ - if (ch == EOF) goto EOL; + if (ch < 0) goto EOL; /* FUDGE: There are sites out there that don't send CRs before their LFs, and other MTAs accept this. We are therefore forced into this "liberalisation" @@ -1959,7 +1962,7 @@ for (;;) prevent further reading), and break out of the loop, having freed the empty header, and set next = NULL to indicate no data line. */ - if (ptr == 0 && ch == '.' && f.dot_ends) + if (f.dot_ends && ptr == 0 && ch == '.') { ch = (receive_getc)(GETC_BUFFER_UNLIMITED); if (ch == '\r') @@ -1967,7 +1970,7 @@ for (;;) ch = (receive_getc)(GETC_BUFFER_UNLIMITED); if (ch != '\n') { - receive_ungetc(ch); + if (ch >= 0) receive_ungetc(ch); ch = '\r'; /* Revert to CR */ } } @@ -2005,7 +2008,7 @@ for (;;) /* Otherwise, put back the character after CR, and turn the bare CR into LF SP. */ - ch = (receive_ungetc)(ch); + if (ch >= 0) (receive_ungetc)(ch); next->text[ptr++] = '\n'; message_size++; ch = ' '; @@ -2089,7 +2092,7 @@ OVERSIZE: whitespace character. If it is, we have a continuation of this header line. There is always space for at least one character at this point. */ - if (ch != EOF) + if (ch >= 0) { int nextch = (receive_getc)(GETC_BUFFER_UNLIMITED); if (nextch == ' ' || nextch == '\t') @@ -2099,8 +2102,9 @@ OVERSIZE: goto OVERSIZE; continue; /* Iterate the loop */ } - else if (nextch != EOF) (receive_ungetc)(nextch); /* For next time */ - else ch = EOF; /* Cause main loop to exit at end */ + else if (nextch >= 0) /* not EOF, ERR etc */ + (receive_ungetc)(nextch); /* For next time */ + else ch = nextch; /* Cause main loop to exit at end */ } /* We have got to the real line end. Terminate the string and release store @@ -2202,7 +2206,7 @@ OVERSIZE: else { - uschar *p = next->text; + uschar * p = next->text; /* If not a valid header line, break from the header reading loop, leaving next != NULL, indicating that it holds the first line of the body. */ @@ -2300,9 +2304,15 @@ OVERSIZE: } /* The line has been handled. If we have hit EOF, break out of the loop, - indicating no pending data line. */ + indicating no pending data line and no more data for the message */ - if (ch == EOF) { next = NULL; break; } + if (ch < 0) + { + next = NULL; + if (ch == EOF) message_ended = END_DOT; + else if (ch == ERR) message_ended = END_PROTOCOL; + break; + } /* Set up for the next header */ @@ -2332,14 +2342,21 @@ DEBUG(D_receive) /* End of file on any SMTP connection is an error. If an incoming SMTP call is dropped immediately after valid headers, the next thing we will see is EOF. We must test for this specially, as further down the reading of the data is -skipped if already at EOF. */ +skipped if already at EOF. +In CHUNKING mode, a protocol error makes us give up on the message. */ -if (smtp_input && (receive_feof)()) - { - smtp_reply = handle_lost_connection(US" (after header)"); - smtp_yield = FALSE; - goto TIDYUP; /* Skip to end of function */ - } +if (smtp_input) + if ((receive_feof)()) + { + smtp_reply = handle_lost_connection(US" (after header)"); + smtp_yield = FALSE; + goto TIDYUP; /* Skip to end of function */ + } + else if (message_ended == END_PROTOCOL) + { + smtp_reply = US""; /* no reply needed */ + goto TIDYUP; + } /* If this is a filter test run and no headers were read, output a warning in case there is a mistake in the test message. */ @@ -4239,7 +4256,8 @@ if ( smtp_input && sender_host_address && !f.sender_host_notsocket if (poll_one_fd(fileno(smtp_in), POLLIN, 0) != 0) { int c = (receive_getc)(GETC_BUFFER_UNLIMITED); - if (c != EOF) (receive_ungetc)(c); else + if (c != EOF) (receive_ungetc)(c); + else { smtp_notquit_exit(US"connection-lost", NULL, NULL); smtp_reply = US""; /* No attempt to send a response */ diff --git a/src/src/smtp_in.c b/src/src/smtp_in.c index 752e80dca..edb0adfaf 100644 --- a/src/src/smtp_in.c +++ b/src/src/smtp_in.c @@ -1757,7 +1757,7 @@ Returns: nothing */ void -smtp_closedown(uschar *message) +smtp_closedown(uschar * message) { if (!smtp_in || smtp_batched_input) return; receive_swallow_smtp(); @@ -3957,6 +3957,8 @@ HAD(SCH_RSET); incomplete_transaction_log(US"RSET"); smtp_printf("250 Reset OK\r\n", FALSE); cmd_list[CMD_LIST_RSET].is_mail_cmd = FALSE; +if (chunking_state > CHUNKING_OFFERED) + chunking_state = CHUNKING_OFFERED; } @@ -5387,7 +5389,7 @@ while (done <= 0) #endif if (!discarded && recipients_count <= 0) { - if (fl.rcpt_smtp_response_same && rcpt_smtp_response != NULL) + if (fl.rcpt_smtp_response_same && rcpt_smtp_response) { uschar *code = US"503"; int len = Ustrlen(rcpt_smtp_response); @@ -6030,7 +6032,6 @@ while (done <= 0) COMMAND_LOOP: last_was_rej_mail = was_rej_mail; /* Remember some last commands for */ last_was_rcpt = was_rcpt; /* protocol error handling */ - continue; } return done - 2; /* Convert yield values */ diff --git a/test/log/0900 b/test/log/0900 index bab750609..f91741a43 100644 --- a/test/log/0900 +++ b/test/log/0900 @@ -13,5 +13,6 @@ 2017-07-30 18:51:05.712 H=(tester) [127.0.0.1] F= rejected RCPT : relay not permitted 2017-07-30 18:51:05.712 H=(tester) [127.0.0.1] F= rejected RCPT : relay not permitted 2017-07-30 18:51:05.712 H=(tester) [127.0.0.1] F= rejected RCPT : relay not permitted +2017-07-30 18:51:05.712 10HmbE-0005vi-00 <= some6ne@some.domain H=(tester) [127.0.0.1] P=esmtp K S=sss for CALLER@test.ex 2017-07-30 18:51:05.712 rejected from H=(tester) [127.0.0.1]: Non-CRLF-terminated header, under CHUNKING: message abandoned -2017-07-30 18:51:05.712 10HmbE-0005vi-00 <= someone@some.domain H=(tester) [127.0.0.1] P=esmtp K S=sss for CALLER@test.ex +2017-07-30 18:51:05.712 10HmbF-0005vi-00 <= someone@some.domain H=(tester) [127.0.0.1] P=esmtp K S=sss for CALLER@test.ex diff --git a/test/scripts/0000-Basic/0900 b/test/scripts/0000-Basic/0900 index a8d2b0539..4edc82952 100644 --- a/test/scripts/0000-Basic/0900 +++ b/test/scripts/0000-Basic/0900 @@ -260,7 +260,7 @@ quit ??? 221 **** # -# Two rejected messages, pipielined, REST between +# Two rejected messages, pipielined, RSET between client 127.0.0.1 PORT_D ??? 220 EHLO tester @@ -296,6 +296,31 @@ quit ??? 221 **** # +# Two messages, pipielined, 1st abandoned midway, RSET between +client 127.0.0.1 PORT_D +??? 220 +EHLO tester +??? 250- +??? 250-SIZE +??? 250-8BITMIME +??? 250-PIPELINING +??? 250-CHUNKING +??? 250 HELP +MAIL FROM:\r\nRCPT TO:\r\nBDAT 86\r\nTo: Susan@random.com\r\nFrom: Sa5@random.com\r\nSubject: This is a bodyless test message\r\nRSET +??? 250 OK +??? 250 Accepted +??? 250 86 byte chunk received +??? 250 Reset OK +MAIL FROM:\r\nRCPT TO:\r\nBDAT 86\r\nTo: Susan@random.com\r\nFrom: Sa6@random.com\r\nSubject: This is a bodyless test message\r\nBDAT 6 LAST\r\nZZ\r\n +??? 250 OK +??? 250 Accepted +??? 250 86 byte chunk received +??? 250- 6 byte chunk, total 93 +??? 250 OK +QUIT +??? 221 +**** +# # # plain, small message (no body) # header line with bad line-ending diff --git a/test/stdout/0900 b/test/stdout/0900 index bcb177fb6..41e369735 100644 --- a/test/stdout/0900 +++ b/test/stdout/0900 @@ -422,6 +422,46 @@ 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:\r\nRCPT TO:\r\nBDAT 86\r\nTo: Susan@random.com\r\nFrom: Sa5@random.com\r\nSubject: This is a bodyless test message\r\nRSET +??? 250 OK +<<< 250 OK +??? 250 Accepted +<<< 250 Accepted +??? 250 86 byte chunk received +<<< 250 86 byte chunk received +??? 250 Reset OK +<<< 250 Reset OK +>>> MAIL FROM:\r\nRCPT TO:\r\nBDAT 86\r\nTo: Susan@random.com\r\nFrom: Sa6@random.com\r\nSubject: This is a bodyless test message\r\nBDAT 6 LAST\r\nZZ\r\n +??? 250 OK +<<< 250 OK +??? 250 Accepted +<<< 250 Accepted +??? 250 86 byte chunk received +<<< 250 86 byte chunk received +??? 250- 6 byte chunk, total 93 +<<< 250- 6 byte chunk, total 93 +??? 250 OK +<<< 250 OK id=10HmbE-0005vi-00 +>>> QUIT +??? 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] @@ -483,7 +523,7 @@ Connecting to 127.0.0.1 port 1225 ... connected ??? 250- <<< 250- 98 byte chunk, total 100 ??? 250 -<<< 250 OK id=10HmbE-0005vi-00 +<<< 250 OK id=10HmbF-0005vi-00 >>> quit ??? 221 <<< 221 testhost.test.ex closing connection -- 2.30.2