Restrict line lengths in bounces. Bug 1760

10 files changed, 227 insertions, 125 deletions

diff --git a/doc/doc-docbook/spec.xfpt b/doc/doc-docbook/spec.xfpt
index cb913d6f1..cd06de8d9 100644
--- a/doc/doc-docbook/spec.xfpt
+++ b/doc/doc-docbook/spec.xfpt
@@ -13786,6 +13786,7 @@ See also the &'Policy controls'& section above.
 .row &%bounce_message_file%&         "content of bounce"
 .row &%bounce_message_text%&         "content of bounce"
 .row &%bounce_return_body%&          "include body if returning message"
+.row &%bounce_return_linesize_limit%& "limit on returned message line length"
 .row &%bounce_return_message%&       "include original message in bounce"
 .row &%bounce_return_size_limit%&    "limit on returned message"
 .row &%bounce_sender_authentication%& "send authenticated sender with bounce"
@@ -14094,6 +14095,24 @@ error that is detected during reception, only those header lines preceding the
 point at which the error was detected are returned.
 .cindex "bounce message" "including original"
 
+.option bounce_return_linesize_limit main integer 998
+.cindex "size" "of bounce lines, limit"
+.cindex "bounce message" "line length limit"
+.cindex "limit" "bounce message line length"
+This option sets a limit in bytes on the line length of messages
+that are returned to senders due to delivery problems,
+when &%bounce_return_message%& is true.
+The default value corresponds to RFC limits.
+If the message being returned has lines longer than this value it is
+treated as if the &%bounce_return_size_limit%& (below) restriction was exceeded.
+
+The option also applies to bounces returned when an error is detected
+during reception of a messsage.
+In this case lines from the original are truncated.
+
+The option does not apply to messages generated by an &(autoreply)& transport.
+
+
 .option bounce_return_message main boolean true
 If this option is set false, none of the original message is included in
 bounce messages generated by Exim. See also &%bounce_return_size_limit%& and
diff --git a/doc/doc-txt/NewStuff b/doc/doc-txt/NewStuff
index e4d1f0607..01bd0111e 100644
--- a/doc/doc-txt/NewStuff
+++ b/doc/doc-txt/NewStuff
@@ -31,6 +31,10 @@ Version 4.87
  7. New base64d and base64 expansion items (the existing str2b64 being a
     synonym of the latter).  Add support in base64 for certificates.
 
+ 8. New main configuration option "bounce_return_linesize_limit" to
+    avoid oversize bodies in bounces. The dafault value matches RFC
+    limits.
+
 
 Version 4.86
 ------------
diff --git a/src/src/deliver.c b/src/src/deliver.c
index 6eb9a65d5..e588ee4a4 100644
--- a/src/src/deliver.c
+++ b/src/src/deliver.c
@@ -7401,6 +7401,9 @@ wording. */
       if (dsn_ret == dsn_ret_hdrs)
         topt |= topt_no_body;
       else
+	{
+	struct stat statbuf;
+
         /* no full body return at all? */
         if (!bounce_return_body)
           {
@@ -7409,16 +7412,18 @@ wording. */
           if (dsn_ret == dsn_ret_full)
             dsnnotifyhdr = dsnlimitmsg;
           }
+	/* line length limited... return headers only if oversize */
         /* size limited ... return headers only if limit reached */
-        else if (bounce_return_size_limit > 0)
-          {
-          struct stat statbuf;
-          if (fstat(deliver_datafile, &statbuf) == 0 && statbuf.st_size > max)
-            {
-              topt |= topt_no_body;
-              dsnnotifyhdr = dsnlimitmsg;
-            }
+	else if (  max_received_linelength > bounce_return_linesize_limit
+		|| (  bounce_return_size_limit > 0
+		   && fstat(deliver_datafile, &statbuf) == 0
+		   && statbuf.st_size > max
+		)  )
+	  {
+	  topt |= topt_no_body;
+	  dsnnotifyhdr = dsnlimitmsg;
           }
+	}
 
 #ifdef SUPPORT_I18N
       if (message_smtputf8)
@@ -7748,6 +7753,7 @@ else if (addr_defer != (address_item *)(+1))
         FILE *wmf = NULL;
         FILE *f = fdopen(fd, "wb");
 	uschar * bound;
+        int topt;
 
         if (warn_message_file)
           if (!(wmf = Ufopen(warn_message_file, "rb")))
@@ -7894,7 +7900,7 @@ else if (addr_defer != (address_item *)(+1))
 
         fflush(f);
         /* header only as required by RFC. only failure DSN needs to honor RET=FULL */
-        int topt = topt_add_return_path | topt_no_body;
+        topt = topt_add_return_path | topt_no_body;
         transport_filter_argv = NULL;   /* Just in case */
         return_path = sender_address;   /* In case not previously set */
         /* Write the original email out */
diff --git a/src/src/globals.c b/src/src/globals.c
index 14a821a19..089d5adb7 100644
--- a/src/src/globals.c
+++ b/src/src/globals.c
@@ -475,6 +475,7 @@ uschar *bounce_message_file    = NULL;
 uschar *bounce_message_text    = NULL;
 uschar *bounce_recipient       = NULL;
 BOOL    bounce_return_body     = TRUE;
+int     bounce_return_linesize_limit = 998;
 BOOL    bounce_return_message  = TRUE;
 int     bounce_return_size_limit = 100*1024;
 uschar *bounce_sender_authentication = NULL;
diff --git a/src/src/globals.h b/src/src/globals.h
index 9ae78a920..bd45e784b 100644
--- a/src/src/globals.h
+++ b/src/src/globals.h
@@ -250,6 +250,7 @@ extern uschar *bounce_message_file;    /* Template file */
 extern uschar *bounce_message_text;    /* One-liner */
 extern uschar *bounce_recipient;       /* When writing an errmsg */
 extern BOOL    bounce_return_body;     /* Include body in returned message */
+extern int     bounce_return_linesize_limit; /* Max line length in return */
 extern BOOL    bounce_return_message;  /* Include message in bounce */
 extern int     bounce_return_size_limit; /* Max amount to return */
 extern uschar *bounce_sender_authentication; /* AUTH address for bounces */
diff --git a/src/src/moan.c b/src/src/moan.c
index e54117c34..7d1a2c681 100644
--- a/src/src/moan.c
+++ b/src/src/moan.c
@@ -86,7 +86,7 @@ else DEBUG(D_any) debug_printf("Child process %d for sending message\n", pid);
 /* Creation of child succeeded */
 
 f = fdopen(fd, "wb");
-if (errors_reply_to != NULL) fprintf(f, "Reply-To: %s\n", errors_reply_to);
+if (errors_reply_to) fprintf(f, "Reply-To: %s\n", errors_reply_to);
 fprintf(f, "Auto-Submitted: auto-replied\n");
 moan_write_from(f);
 fprintf(f, "To: %s\n", recipient);
@@ -94,140 +94,137 @@ fprintf(f, "To: %s\n", recipient);
 switch(ident)
   {
   case ERRMESS_BADARGADDRESS:
-  fprintf(f,
-  "Subject: Mail failure - malformed recipient address\n\n");
-  fprintf(f,
-  "A message that you sent contained a recipient address that was incorrectly\n"
-  "constructed:\n\n");
-  fprintf(f, "  %s  %s\n", eblock->text1, eblock->text2);
-  count = Ustrlen(eblock->text1);
-  if (count > 0 && eblock->text1[count-1] == '.')
     fprintf(f,
-    "\nRecipient addresses must not end with a '.' character.\n");
-  fprintf(f,
-  "\nThe message has not been delivered to any recipients.\n");
-  break;
+      "Subject: Mail failure - malformed recipient address\n\n");
+    fprintf(f,
+      "A message that you sent contained a recipient address that was incorrectly\n"
+      "constructed:\n\n");
+    fprintf(f, "  %s  %s\n", eblock->text1, eblock->text2);
+    count = Ustrlen(eblock->text1);
+    if (count > 0 && eblock->text1[count-1] == '.')
+      fprintf(f,
+	"\nRecipient addresses must not end with a '.' character.\n");
+    fprintf(f,
+      "\nThe message has not been delivered to any recipients.\n");
+    break;
 
   case ERRMESS_BADNOADDRESS:
   case ERRMESS_BADADDRESS:
-  fprintf(f,
-  "Subject: Mail failure - malformed recipient address\n\n");
-  fprintf(f,
-  "A message that you sent contained one or more recipient addresses that were\n"
-  "incorrectly constructed:\n\n");
+    fprintf(f,
+      "Subject: Mail failure - malformed recipient address\n\n");
+    fprintf(f,
+      "A message that you sent contained one or more recipient addresses that were\n"
+      "incorrectly constructed:\n\n");
 
-  while (eblock != NULL)
-    {
-    fprintf(f, "  %s: %s\n", eblock->text1, eblock->text2);
-    count++;
-    eblock = eblock->next;
-    }
+    while (eblock != NULL)
+      {
+      fprintf(f, "  %s: %s\n", eblock->text1, eblock->text2);
+      count++;
+      eblock = eblock->next;
+      }
 
-  fprintf(f, (count == 1)? "\nThis address has been ignored. " :
-    "\nThese addresses have been ignored. ");
+    fprintf(f, (count == 1)? "\nThis address has been ignored. " :
+      "\nThese addresses have been ignored. ");
 
-  fprintf(f, (ident == ERRMESS_BADADDRESS)?
-  "The other addresses in the message were\n"
-  "syntactically valid and have been passed on for an attempt at delivery.\n" :
+    fprintf(f, (ident == ERRMESS_BADADDRESS)?
+      "The other addresses in the message were\n"
+      "syntactically valid and have been passed on for an attempt at delivery.\n" :
 
-  "There were no other addresses in your\n"
-  "message, and so no attempt at delivery was possible.\n");
-  break;
+      "There were no other addresses in your\n"
+      "message, and so no attempt at delivery was possible.\n");
+    break;
 
   case ERRMESS_IGADDRESS:
-  fprintf(f, "Subject: Mail failure - no recipient addresses\n\n");
-  fprintf(f,
-  "A message that you sent using the -t command line option contained no\n"
-  "addresses that were not also on the command line, and were therefore\n"
-  "suppressed. This left no recipient addresses, and so no delivery could\n"
-  "be attempted.\n");
-  break;
+    fprintf(f, "Subject: Mail failure - no recipient addresses\n\n");
+    fprintf(f,
+      "A message that you sent using the -t command line option contained no\n"
+      "addresses that were not also on the command line, and were therefore\n"
+      "suppressed. This left no recipient addresses, and so no delivery could\n"
+      "be attempted.\n");
+    break;
 
   case ERRMESS_NOADDRESS:
-  fprintf(f, "Subject: Mail failure - no recipient addresses\n\n");
-  fprintf(f,
-  "A message that you sent contained no recipient addresses, and therefore no\n"
-  "delivery could be attempted.\n");
-  break;
+    fprintf(f, "Subject: Mail failure - no recipient addresses\n\n");
+    fprintf(f,
+      "A message that you sent contained no recipient addresses, and therefore no\n"
+      "delivery could be attempted.\n");
+    break;
 
   case ERRMESS_IOERR:
-  fprintf(f, "Subject: Mail failure - system failure\n\n");
-  fprintf(f,
-  "A system failure was encountered while processing a message that you sent,\n"
-  "so it has not been possible to deliver it. The error was:\n\n%s\n",
-    eblock->text1);
-  break;
+    fprintf(f, "Subject: Mail failure - system failure\n\n");
+    fprintf(f,
+      "A system failure was encountered while processing a message that you sent,\n"
+      "so it has not been possible to deliver it. The error was:\n\n%s\n",
+      eblock->text1);
+    break;
 
   case ERRMESS_VLONGHEADER:
-  fprintf(f, "Subject: Mail failure - overlong header section\n\n");
-  fprintf(f,
-  "A message that you sent contained a header section that was excessively\n"
-  "long and could not be handled by the mail transmission software. The\n"
-  "message has not been delivered to any recipients.\n");
-  break;
+    fprintf(f, "Subject: Mail failure - overlong header section\n\n");
+    fprintf(f,
+      "A message that you sent contained a header section that was excessively\n"
+      "long and could not be handled by the mail transmission software. The\n"
+      "message has not been delivered to any recipients.\n");
+    break;
 
   case ERRMESS_VLONGHDRLINE:
-  fprintf(f, "Subject: Mail failure - overlong header line\n\n");
-  fprintf(f,
-  "A message that you sent contained a header line that was excessively\n"
-  "long and could not be handled by the mail transmission software. The\n"
-  "message has not been delivered to any recipients.\n");
-  break;
+    fprintf(f, "Subject: Mail failure - overlong header line\n\n");
+    fprintf(f,
+      "A message that you sent contained a header line that was excessively\n"
+      "long and could not be handled by the mail transmission software. The\n"
+      "message has not been delivered to any recipients.\n");
+    break;
 
   case ERRMESS_TOOBIG:
-  fprintf(f, "Subject: Mail failure - message too big\n\n");
-  fprintf(f,
-  "A message that you sent was longer than the maximum size allowed on this\n"
-  "system. It was not delivered to any recipients.\n");
-  break;
+    fprintf(f, "Subject: Mail failure - message too big\n\n");
+    fprintf(f,
+      "A message that you sent was longer than the maximum size allowed on this\n"
+      "system. It was not delivered to any recipients.\n");
+    break;
 
   case ERRMESS_TOOMANYRECIP:
-  fprintf(f, "Subject: Mail failure - too many recipients\n\n");
-  fprintf(f,
-  "A message that you sent contained more recipients than allowed on this\n"
-  "system. It was not delivered to any recipients.\n");
-  break;
+    fprintf(f, "Subject: Mail failure - too many recipients\n\n");
+    fprintf(f,
+      "A message that you sent contained more recipients than allowed on this\n"
+      "system. It was not delivered to any recipients.\n");
+    break;
 
   case ERRMESS_LOCAL_SCAN:
   case ERRMESS_LOCAL_ACL:
-  fprintf(f, "Subject: Mail failure - rejected by local scanning code\n\n");
-  fprintf(f,
-  "A message that you sent was rejected by the local scanning code that\n"
-  "checks incoming messages on this system.");
-  if (eblock->text1 != NULL)
-    {
+    fprintf(f, "Subject: Mail failure - rejected by local scanning code\n\n");
     fprintf(f,
-    " The following error was given:\n\n  %s", eblock->text1);
-    }
+      "A message that you sent was rejected by the local scanning code that\n"
+      "checks incoming messages on this system.");
+      if (eblock->text1)
+	fprintf(f, " The following error was given:\n\n  %s", eblock->text1);
   fprintf(f, "\n");
   break;
 
 #ifdef EXPERIMENTAL_DMARC
   case ERRMESS_DMARC_FORENSIC:
-  bounce_return_message = TRUE;
-  bounce_return_body    = FALSE;
-  fprintf(f,
+    bounce_return_message = TRUE;
+    bounce_return_body    = FALSE;
+    fprintf(f,
           "Subject: DMARC Forensic Report for %s from IP %s\n\n",
 	  ((eblock == NULL) ? US"Unknown" : eblock->text2),
           sender_host_address);
-  fprintf(f,
-  "A message claiming to be from you has failed the published DMARC\n"
-  "policy for your domain.\n\n");
-  while (eblock != NULL)
-    {
-    fprintf(f, "  %s: %s\n", eblock->text1, eblock->text2);
-    count++;
-    eblock = eblock->next;
-    }
+    fprintf(f,
+      "A message claiming to be from you has failed the published DMARC\n"
+      "policy for your domain.\n\n");
+    while (eblock != NULL)
+      {
+      fprintf(f, "  %s: %s\n", eblock->text1, eblock->text2);
+      count++;
+      eblock = eblock->next;
+      }
   break;
 #endif
 
   default:
-  fprintf(f, "Subject: Mail failure\n\n");
-  fprintf(f,
-  "A message that you sent has caused the error routine to be entered with\n"
-  "an unknown error number (%d).\n", ident);
-  break;
+    fprintf(f, "Subject: Mail failure\n\n");
+    fprintf(f,
+      "A message that you sent has caused the error routine to be entered with\n"
+      "an unknown error number (%d).\n", ident);
+    break;
   }
 
 /* Now, if configured, copy the message; first the headers and then the rest of
@@ -267,7 +264,7 @@ if (bounce_return_message)
   /* If the error occurred before the Received: header was created, its text
   field will still be NULL; just omit such a header line. */
 
-  while (headers != NULL)
+  while (headers)
     {
     if (headers->text != NULL) fprintf(f, "%s", CS headers->text);
     headers = headers->next;
@@ -280,30 +277,47 @@ if (bounce_return_message)
   in which case we might have to terminate on a line containing just "."
   as well as on EOF. We may already have the first line in memory. */
 
-  if (bounce_return_body && message_file != NULL)
+  if (bounce_return_body && message_file)
     {
     int ch;
-    int state = 1;
+    enum {midline, beginline, haddot} state = beginline;
     BOOL enddot = dot_ends && message_file == stdin;
-    if (firstline != NULL) fprintf(f, "%s", CS firstline);
-    while ((ch = fgetc(message_file)) != EOF)
+    uschar * buf = store_get(bounce_return_linesize_limit+2);
+
+    if (firstline) fprintf(f, "%s", CS firstline);
+
+    while (fgets(buf, bounce_return_linesize_limit+2, message_file))
       {
-      fputc(ch, f);
-      if (size_limit > 0 && ++written > size_limit) break;
-      if (enddot)
-        {
-        if (state == 0) { if (ch == '\n') state = 1; }
-        else if (state == 1)
-          { if (ch == '.') state = 2; else if (ch != '\n') state = 0; }
-        else
-          { if (ch == '\n') break; else state = 0; }
-        }
+      int len;
+
+      if (enddot && *buf == '.' && buf[1] == '\n')
+	{
+	fputc('.', f);
+	break;
+	}
+
+      len = Ustrlen(buf);
+      if (buf[len-1] != '\n')
+	{	/* eat rest of partial line */
+	int ch;
+	while ((ch = fgetc(message_file)) != EOF && ch != '\n') ;
+	}
+
+      if (size_limit > 0 && len > size_limit - written)
+	{
+	buf[size_limit - written] = '\0';
+	fputs(buf, f);
+	break;
+	}
+
+      fputs(buf, f);
       }
     }
 #ifdef EXPERIMENTAL_DMARC
   /* Overkill, but use exact test in case future code gets inserted */
   else if (bounce_return_body && message_file == NULL)
     {
+    /*XXX limit line length here? */
     /* This doesn't print newlines, disable until can parse and fix
      * output to be legible.  */
     fprintf(f, "%s", expand_string(US"$message_body"));
@@ -365,24 +379,24 @@ moan_to_sender(int ident, error_block *eblock, header_line *headers,
 uschar *firstline = NULL;
 uschar *msg = US"Error while reading message with no usable sender address";
 
-if (message_reference != NULL)
+if (message_reference)
   msg = string_sprintf("%s (R=%s)", msg, message_reference);
 
 /* Find the sender from a From line if permitted and possible */
 
-if (check_sender && message_file != NULL && trusted_caller &&
+if (check_sender && message_file && trusted_caller &&
     Ufgets(big_buffer, BIG_BUFFER_SIZE, message_file) != NULL)
   {
   uschar *new_sender = NULL;
   if (regex_match_and_setup(regex_From, big_buffer, 0, -1))
     new_sender = expand_string(uucp_from_sender);
-  if (new_sender != NULL) sender_address = new_sender;
+  if (new_sender) sender_address = new_sender;
     else firstline = big_buffer;
   }
 
 /* If viable sender address, send a message */
 
-if (sender_address != NULL && sender_address[0] != 0 && !local_error_message)
+if (sender_address && sender_address[0] && !local_error_message)
   return moan_send_message(sender_address, ident, eblock, headers,
     message_file, firstline);
 
diff --git a/src/src/readconf.c b/src/src/readconf.c
index e52f45fca..f90b66c3d 100644
--- a/src/src/readconf.c
+++ b/src/src/readconf.c
@@ -194,6 +194,7 @@ static optionlist optionlist_config[] = {
   { "bounce_message_file",      opt_stringptr,   &bounce_message_file },
   { "bounce_message_text",      opt_stringptr,   &bounce_message_text },
   { "bounce_return_body",       opt_bool,        &bounce_return_body },
+  { "bounce_return_linesize_limit", opt_mkint,   &bounce_return_linesize_limit },
   { "bounce_return_message",    opt_bool,        &bounce_return_message },
   { "bounce_return_size_limit", opt_mkint,       &bounce_return_size_limit },
   { "bounce_sender_authentication",opt_stringptr,&bounce_sender_authentication },
diff --git a/test/confs/0001 b/test/confs/0001
index 2b53c0942..4b7ea85f6 100644
--- a/test/confs/0001
+++ b/test/confs/0001
@@ -38,6 +38,7 @@ bounce_return_body = false
 no_bounce_return_message
 return_size_limit = 12K
 bounce_return_size_limit = 10K
+bounce_return_line_limit = 997
 callout_domain_negative_expire = 1h
 callout_domain_positive_expire = 1d
 callout_negative_expire = 5h
diff --git a/test/confs/0573 b/test/confs/0573
new file mode 100644
index 000000000..c3c7a6bea
--- /dev/null
+++ b/test/confs/0573
@@ -0,0 +1,37 @@
+# Exim test configuration 0573
+
+exim_path = EXIM_PATH
+hide host_lookup_order = bydns
+primary_hostname = myhost.test.ex
+spool_directory = DIR/spool
+log_file_path = DIR/spool/log/%slog
+gecos_pattern = ""
+gecos_name = CALLER_NAME
+tls_advertise_hosts =
+
+# ----- Main settings -----
+
+trusted_users = CALLER
+bounce_return_linesize_limit = 20
+acl_smtp_rcpt = accept
+
+
+# ----- Routers -----
+
+begin routers
+
+my_main_router:
+  driver = accept
+  domains = myhost.test.ex
+  transport = t1
+
+# ----- Transports -----
+
+begin transports
+
+t1:
+  driver = appendfile
+  file = DIR/test-mail/$local_part
+  user = CALLER
+
+# End
diff --git a/test/scripts/0000-Basic/0573 b/test/scripts/0000-Basic/0573
new file mode 100644
index 000000000..88606d23f
--- /dev/null
+++ b/test/scripts/0000-Basic/0573
@@ -0,0 +1,18 @@
+# bounce_return_linesize_limit
+#
+exim -odi -f not_limited fred@undeliverable.org
+Subject: test
+
+msg with ok lines
+00000000001111111
+01234567890123456
+****
+#
+exim -odi -f limited fred@undeliverable.org
+Subject: test
+
+msg with long lines
+000000000011111111112222222222
+012345678901234567890123456789
+****
+no_msglog_check