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.

diff --git a/doc/doc-docbook/spec.xfpt b/doc/doc-docbook/spec.xfpt
index 64807899824d0b976d5fc6388182364c7c7cb6d7..422edcee47e65d27def9cf6cc86283a8aecc37e9 100644 (file)
--- 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.
 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.
 
 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
 .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
 .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 9c7dbe6716762d9cd40d46ead70fe33b82fa076b..eeeb33390a46ff0db9df45c597bdba24aace7fee 100644 (file)
--- 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 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 *);
 
 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 48cdff88c711b0f6a133a98af7761d9f0429c374..6ce2823f514d186654ee9055379e57819a94a42b 100644 (file)
--- 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
 
 
 /* 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
 
 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)
 {
 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*/
 }
   }
 /*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. */
 
   /* 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");
 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");
   }
     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. */
 
 /* 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;
   {
   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 */
 
 
   /* 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))
     {
     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. */
 
 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);
   {
   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 8f43096863eaaa5d737383c31366632954fc4dfe..fa4bb99ed78acfd27a1f812ff9956ff9e7e67177 100644 (file)
--- 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)
 bdat_flush_data(void)
 {
 while (chunking_data_left > 0)

diff --git a/test/log/0900 b/test/log/0900
index 8ce3bcb0f59f06eb8e3296a379d055dcec7b346a..cf02d983a89faea7718a21b951c60bbf7130d33c 100644 (file)
--- 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 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 4c194b51018fc7e92566a8dfc980fb7eabedf0bf..e5eb296d59006c0996a285a05214851c5c3220e1 100644 (file)
--- 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
 
 ******** 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 9e014c43e60594db9b12c0b5f7cdc2301a964cde..4503ae0c01b9419212bd4cc6f47cca9456878d9b 100644 (file)
--- a/test/scripts/0000-Basic/0900
+++ b/test/scripts/0000-Basic/0900
@@ -215,5 +215,31 @@ quit
 ??? 221
 ****
 #
 ??? 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
 killdaemon
 no_msglog_check

diff --git a/test/stdout/0900 b/test/stdout/0900
index 70c527c17fd6ee78c41e945301589e7b5f4a6559..72269fad92bff079a58e4f3a81b4162ac2a17e99 100644 (file)
--- 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
 ??? 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