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.

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=<someone@some.domain> rejected RCPT <dummy@reject.ex>: relay not permitted
+1999-03-02 09:44:33 rejected from <someone@some.domain> 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=<someone@some.domain> rejected RCPT <dummy@reject.ex>: relay not permitted
+1999-03-02 09:44:33 rejected from <someone@some.domain> H=(tester) [127.0.0.1]: Non-CRLF-terminated header, under CHUNKING: message abandoned
+Envelope-from: <someone@some.domain>
+Envelope-to: <CALLER@test.ex>
+  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