Sort out group syntax problems, particularly with verify=header_sender.
authorPhilip Hazel <ph10@hermes.cam.ac.uk>
Tue, 10 Oct 2006 15:36:50 +0000 (15:36 +0000)
committerPhilip Hazel <ph10@hermes.cam.ac.uk>
Tue, 10 Oct 2006 15:36:50 +0000 (15:36 +0000)
doc/doc-txt/ChangeLog
src/src/parse.c
src/src/receive.c
src/src/sieve.c
src/src/verify.c
test/log/0026
test/mail/0026.userx
test/rejectlog/0026
test/scripts/0000-Basic/0026
test/stderr/0026
test/stdout/0026

index 6d25eec705dc2365dfa3d18bee666733bd8fb4eb..e09227ded748d3154f334c047869dcebe2e31e3c 100644 (file)
@@ -1,4 +1,4 @@
-$Cambridge: exim/doc/doc-txt/ChangeLog,v 1.405 2006/10/10 11:15:12 ph10 Exp $
+$Cambridge: exim/doc/doc-txt/ChangeLog,v 1.406 2006/10/10 15:36:50 ph10 Exp $
 
 Change log file for Exim from version 4.21
 -------------------------------------------
@@ -117,6 +117,17 @@ PH/17 Applied (a modified version of) Nico Erfurth's patch to make
       str(n)cmp tests so they don't re-test the leading "-" and the first
       character, in the hope this might squeeze out yet more improvement.
 
+PH/18 Two problems with "group" syntax in header lines when verifying: (1) The
+      flag allowing group syntax was set by the header_syntax check but not
+      turned off, possible causing trouble later; (2) The flag was not being
+      set at all for the header_verify test, causing "group"-style headers to
+      be rejected. I have now set it in this case, and also caused header_
+      verify to ignore an empty address taken from a group. While doing this, I
+      came across some other cases where the code for allowing group syntax
+      while scanning a header line wasn't quite right (mostly, not resetting
+      the flag correctly in the right place). These bugs could have caused
+      trouble for malformed header lines. I hope it is now all correct.
+
 
 Exim version 4.63
 -----------------
index a0366431e2a07e7fc8ce77852c1be012ad094217..0c1b9fe8f673d792bd2054ea5d084d539ce17c8f 100644 (file)
@@ -1,4 +1,4 @@
-/* $Cambridge: exim/src/src/parse.c,v 1.9 2006/03/08 11:13:07 ph10 Exp $ */
+/* $Cambridge: exim/src/src/parse.c,v 1.10 2006/10/10 15:36:50 ph10 Exp $ */
 
 /*************************************************
 *     Exim - an Internet mail transport agent    *
@@ -597,10 +597,15 @@ which may appear in certain headers. If the flag parse_allow_group is set
 TRUE and parse_found_group is FALSE when this function is called, an address
 which is the start of a group (i.e. preceded by a phrase and a colon) is
 recognized; the phrase is ignored and the flag parse_found_group is set. If
-this flag is TRUE at the end of an address, then if an extraneous semicolon is
-found, it is ignored and the flag is cleared. This logic is used only when
-scanning through addresses in headers, either to fulfil the -t option or for
-rewriting or checking header syntax.
+this flag is TRUE at the end of an address, and if an extraneous semicolon is
+found, it is ignored and the flag is cleared.
+
+This logic is used only when scanning through addresses in headers, either to
+fulfil the -t option, or for rewriting, or for checking header syntax. Because
+the group "state" has to be remembered between multiple calls of this function,
+the variables parse_{allow,found}_group are global. It is important to ensure
+that they are reset to FALSE at the end of scanning a header's list of
+addresses.
 
 Arguments:
   mailbox     points to the RFC822 mailbox
index 797444ca1b3b6963c5a2cbecfc45e6be9c9ae96f..73675bee209f44a37f7fc045a3f8788737200013 100644 (file)
@@ -1,4 +1,4 @@
-/* $Cambridge: exim/src/src/receive.c,v 1.29 2006/09/25 10:14:20 ph10 Exp $ */
+/* $Cambridge: exim/src/src/receive.c,v 1.30 2006/10/10 15:36:50 ph10 Exp $ */
 
 /*************************************************
 *     Exim - an Internet mail transport agent    *
@@ -2051,8 +2051,6 @@ if (extract_recip)
     recipients_count = recipients_list_max = 0;
     }
 
-  parse_allow_group = TRUE;             /* Allow address group syntax */
-
   /* Now scan the headers */
 
   for (h = header_list->next; h != NULL; h = h->next)
@@ -2063,6 +2061,8 @@ if (extract_recip)
       uschar *s = Ustrchr(h->text, ':') + 1;
       while (isspace(*s)) s++;
 
+      parse_allow_group = TRUE;          /* Allow address group syntax */
+
       while (*s != 0)
         {
         uschar *ss = parse_find_address_end(s, FALSE);
@@ -2127,7 +2127,10 @@ if (extract_recip)
 
         s = ss + (*ss? 1:0);
         while (isspace(*s)) s++;
-        }
+        }    /* Next address */
+
+      parse_allow_group = FALSE;      /* Reset group syntax flags */
+      parse_found_group = FALSE;
 
       /* If this was the bcc: header, mark it "old", which means it
       will be kept on the spool, but not transmitted as part of the
@@ -2137,8 +2140,6 @@ if (extract_recip)
       }   /* For appropriate header line */
     }     /* For each header line */
 
-  parse_allow_group = FALSE;      /* Reset group syntax flags */
-  parse_found_group = FALSE;
   }
 
 /* Now build the unique message id. This has changed several times over the
index 425a0b9e0a4304ad17bea1f59f6e037b47bf34c7..549dba197c3578f3513ad9f195a0f789d3adddca 100644 (file)
@@ -1,4 +1,4 @@
-/* $Cambridge: exim/src/src/sieve.c,v 1.22 2006/09/05 14:05:43 ph10 Exp $ */
+/* $Cambridge: exim/src/src/sieve.c,v 1.23 2006/10/10 15:36:50 ph10 Exp $ */
 
 /*************************************************
 *     Exim - an Internet mail transport agent    *
@@ -1826,6 +1826,8 @@ if (parse_identifier(filter,CUS "address"))
         if (saveend == 0) break;
         header_value = end_addr + 1;
         }
+      parse_allow_group = FALSE;
+      parse_found_group = FALSE;
       }
     }
   return 1;
index 4f0da9bbdf65c1792ba72ad5e829560d63e601db..5b635989eb7ce169d6b10b9b86bc58e534d52adc 100644 (file)
@@ -1,4 +1,4 @@
-/* $Cambridge: exim/src/src/verify.c,v 1.42 2006/10/09 14:36:25 ph10 Exp $ */
+/* $Cambridge: exim/src/src/verify.c,v 1.43 2006/10/10 15:36:50 ph10 Exp $ */
 
 /*************************************************
 *     Exim - an Internet mail transport agent    *
@@ -1451,8 +1451,9 @@ verify_check_headers(uschar **msgptr)
 {
 header_line *h;
 uschar *colon, *s;
+int yield = OK;
 
-for (h = header_list; h != NULL; h = h->next)
+for (h = header_list; h != NULL && yield == OK; h = h->next)
   {
   if (h->type != htype_from &&
       h->type != htype_reply_to &&
@@ -1466,9 +1467,10 @@ for (h = header_list; h != NULL; h = h->next)
   s = colon + 1;
   while (isspace(*s)) s++;
 
-  parse_allow_group = TRUE;     /* Allow group syntax */
+  /* Loop for multiple addresses in the header, enabling group syntax. Note
+  that we have to reset this after the header has been scanned. */
 
-  /* Loop for multiple addresses in the header */
+  parse_allow_group = TRUE;
 
   while (*s != 0)
     {
@@ -1478,7 +1480,7 @@ for (h = header_list; h != NULL; h = h->next)
     int start, end, domain;
 
     /* Temporarily terminate the string at this point, and extract the
-    operative address within. */
+    operative address within, allowing group syntax. */
 
     *ss = 0;
     recipient = parse_extract_address(s,&errmess,&start,&end,&domain,FALSE);
@@ -1534,7 +1536,8 @@ for (h = header_list; h != NULL; h = h->next)
         string_sprintf("%s: failing address in \"%.*s:\" header %s: %.*s",
           errmess, tt - h->text, h->text, verb, len, s));
 
-      return FAIL;
+      yield = FAIL;
+      break;          /* Out of address loop */
       }
 
     /* Advance to the next address */
@@ -1542,9 +1545,12 @@ for (h = header_list; h != NULL; h = h->next)
     s = ss + (terminator? 1:0);
     while (isspace(*s)) s++;
     }   /* Next address */
-  }     /* Next header */
 
-return OK;
+  parse_allow_group = FALSE;
+  parse_found_group = FALSE;
+  }     /* Next header unless yield has been set FALSE */
+
+return yield;
 }
 
 
@@ -1587,9 +1593,10 @@ for (i = 0; i < recipients_count; i++)
     s = colon + 1;
     while (isspace(*s)) s++;
 
-    parse_allow_group = TRUE;     /* Allow group syntax */
+    /* Loop for multiple addresses in the header, enabling group syntax. Note
+    that we have to reset this after the header has been scanned. */
 
-    /* Loop for multiple addresses in the header */
+    parse_allow_group = TRUE;
 
     while (*s != 0)
       {
@@ -1599,7 +1606,7 @@ for (i = 0; i < recipients_count; i++)
       int start, end, domain;
 
       /* Temporarily terminate the string at this point, and extract the
-      operative address within. */
+      operative address within, allowing group syntax. */
 
       *ss = 0;
       recipient = parse_extract_address(s,&errmess,&start,&end,&domain,FALSE);
@@ -1623,6 +1630,9 @@ for (i = 0; i < recipients_count; i++)
       s = ss + (terminator? 1:0);
       while (isspace(*s)) s++;
       }   /* Next address */
+
+    parse_allow_group = FALSE;
+    parse_found_group = FALSE;
     }     /* Next header (if found is false) */
 
   if (!found) return FAIL;
@@ -1705,13 +1715,14 @@ verify_check_header_address(uschar **user_msgptr, uschar **log_msgptr,
   uschar *pm_mailfrom, int options, int *verrno)
 {
 static int header_types[] = { htype_sender, htype_reply_to, htype_from };
+BOOL done = FALSE;
 int yield = FAIL;
 int i;
 
-for (i = 0; i < 3; i++)
+for (i = 0; i < 3 && !done; i++)
   {
   header_line *h;
-  for (h = header_list; h != NULL; h = h->next)
+  for (h = header_list; h != NULL && !done; h = h->next)
     {
     int terminator, new_ok;
     uschar *s, *ss, *endname;
@@ -1719,6 +1730,11 @@ for (i = 0; i < 3; i++)
     if (h->type != header_types[i]) continue;
     s = endname = Ustrchr(h->text, ':') + 1;
 
+    /* Scan the addresses in the header, enabling group syntax. Note that we
+    have to reset this after the header has been scanned. */
+
+    parse_allow_group = TRUE;
+
     while (*s != 0)
       {
       address_item *vaddr;
@@ -1761,11 +1777,21 @@ for (i = 0; i < 3; i++)
       else
         {
         int start, end, domain;
-        uschar *address = parse_extract_address(s, log_msgptr, &start,
-          &end, &domain, FALSE);
+        uschar *address = parse_extract_address(s, log_msgptr, &start, &end,
+          &domain, FALSE);
 
         *ss = terminator;
 
+        /* If we found an empty address, just carry on with the next one, but
+        kill the message. */
+
+        if (address == NULL && Ustrcmp(*log_msgptr, "empty address") == 0)
+          {
+          *log_msgptr = NULL;
+          s = ss;
+          continue;
+          }
+
         /* If verification failed because of a syntax error, fail this
         function, and ensure that the failing address gets added to the error
         message. */
@@ -1773,14 +1799,13 @@ for (i = 0; i < 3; i++)
         if (address == NULL)
           {
           new_ok = FAIL;
-          if (*log_msgptr != NULL)
-            {
-            while (ss > s && isspace(ss[-1])) ss--;
-            *log_msgptr = string_sprintf("syntax error in '%.*s' header when "
-              "scanning for sender: %s in \"%.*s\"",
-              endname - h->text, h->text, *log_msgptr, ss - s, s);
-            return FAIL;
-            }
+          while (ss > s && isspace(ss[-1])) ss--;
+          *log_msgptr = string_sprintf("syntax error in '%.*s' header when "
+            "scanning for sender: %s in \"%.*s\"",
+            endname - h->text, h->text, *log_msgptr, ss - s, s);
+          yield = FAIL;
+          done = TRUE;
+          break;
           }
 
         /* Else go ahead with the sender verification. But it isn't *the*
@@ -1814,15 +1839,24 @@ for (i = 0; i < 3; i++)
 
       /* Success or defer */
 
-      if (new_ok == OK) return OK;
+      if (new_ok == OK)
+        {
+        yield = OK;
+        done = TRUE;
+        break;
+        }
+
       if (new_ok == DEFER) yield = DEFER;
 
       /* Move on to any more addresses in the header */
 
       s = ss;
-      }
-    }
-  }
+      }     /* Next address */
+
+    parse_allow_group = FALSE;
+    parse_found_group = FALSE;
+    }       /* Next header, unless done */
+  }         /* Next header type unless done */
 
 if (yield == FAIL && *log_msgptr == NULL)
   *log_msgptr = US"there is no valid sender in any header line";
index be92f738ad6b7387c87c9c9c0ccef61fbb765718..cf9958438cd599a4e852524657f4cd4f0d12e465 100644 (file)
@@ -1,12 +1,16 @@
-1999-03-02 09:44:33 10HmbB-0005vi-00 <= x@y U=CALLER P=local-smtp S=sss
+1999-03-02 09:44:33 10HmbC-0005vi-00 <= x@y U=CALLER P=local-smtp S=sss
 1999-03-02 09:44:33 10HmaX-0005vi-00 U=CALLER F=<x@y> rejected after DATA: domain missing or malformed: failing address in "From:" header is: @
 1999-03-02 09:44:33 10HmaY-0005vi-00 U=CALLER F=<> rejected after DATA: domain missing or malformed: failing address in "From:" header is: @
 1999-03-02 09:44:33 10HmaZ-0005vi-00 U=CALLER F=<> rejected after DATA: there is no valid sender in any header line
-1999-03-02 09:44:33 10HmbC-0005vi-00 <= <> U=CALLER P=local-smtp S=sss
+1999-03-02 09:44:33 10HmbD-0005vi-00 <= <> U=CALLER P=local-smtp S=sss
 1999-03-02 09:44:33 10HmbA-0005vi-00 U=CALLER F=<x@y> rejected after DATA: body contains trigger
-1999-03-02 09:44:33 10HmbD-0005vi-00 <= x@y U=CALLER P=local-smtp S=sss
-1999-03-02 09:44:33 10HmbD-0005vi-00 => userx <userx@test.ex> R=r2 T=local_delivery
-1999-03-02 09:44:33 10HmbD-0005vi-00 Completed
 1999-03-02 09:44:33 10HmbE-0005vi-00 <= x@y U=CALLER P=local-smtp S=sss
 1999-03-02 09:44:33 10HmbE-0005vi-00 => userx <userx@test.ex> R=r2 T=local_delivery
 1999-03-02 09:44:33 10HmbE-0005vi-00 Completed
+1999-03-02 09:44:33 10HmbF-0005vi-00 <= x@y U=CALLER P=local-smtp S=sss
+1999-03-02 09:44:33 10HmbF-0005vi-00 => userx <userx@test.ex> R=r2 T=local_delivery
+1999-03-02 09:44:33 10HmbF-0005vi-00 Completed
+1999-03-02 09:44:33 10HmbG-0005vi-00 <= <> U=CALLER P=local-smtp S=sss
+1999-03-02 09:44:33 10HmbG-0005vi-00 => userx <userx@test.ex> R=r2 T=local_delivery
+1999-03-02 09:44:33 10HmbG-0005vi-00 Completed
+1999-03-02 09:44:33 10HmbB-0005vi-00 U=CALLER F=<> rejected after DATA: there is no valid sender in any header line
index 74f68e2cb03461b489cdcec0358aade7dc46c52a..4c78221971d8fdce03010f9559713749e9ce97c1 100644 (file)
@@ -1,9 +1,9 @@
 From x@y Tue Mar 02 09:44:33 1999
 Received: from CALLER by myhost.test.ex with local-smtp (Exim x.yz)
        (envelope-from <x@y>)
-       id 10HmbD-0005vi-00
+       id 10HmbE-0005vi-00
        for userx@test.ex; Tue, 2 Mar 1999 09:44:33 +0000
-Message-Id: <E10HmbD-0005vi-00@myhost.test.ex>
+Message-Id: <E10HmbE-0005vi-00@myhost.test.ex>
 From: x@y
 Date: Tue, 2 Mar 1999 09:44:33 +0000
 X-warning: this is a test warning
@@ -13,14 +13,27 @@ Message 7
 From x@y Tue Mar 02 09:44:33 1999
 Received: from CALLER by myhost.test.ex with local-smtp (Exim x.yz)
        (envelope-from <x@y>)
-       id 10HmbE-0005vi-00
+       id 10HmbF-0005vi-00
        for userx@test.ex; Tue, 2 Mar 1999 09:44:33 +0000
 to: group name: x@y, p@q;
 reply-to: group name: a@b, c@d;
-Message-Id: <E10HmbE-0005vi-00@myhost.test.ex>
+Message-Id: <E10HmbF-0005vi-00@myhost.test.ex>
 From: x@y
 Date: Tue, 2 Mar 1999 09:44:33 +0000
 X-warning: this is a test warning
 
 Message 10
 
+From MAILER-DAEMON Tue Mar 02 09:44:33 1999
+Received: from CALLER by myhost.test.ex with local-smtp (Exim x.yz)
+       id 10HmbG-0005vi-00
+       for userx@test.ex; Tue, 2 Mar 1999 09:44:33 +0000
+to: group name: x@y, p@q;
+reply-to: group name:;
+from: userx@test.ex
+Message-Id: <E10HmbG-0005vi-00@myhost.test.ex>
+Date: Tue, 2 Mar 1999 09:44:33 +0000
+X-warning: this is a test warning
+
+Message 11
+
index 384d6ef6e77d5cbc3d5dad34f91e71d71d3a44ab..716e35cf0006653e71d44ce6bd4cdd5542bf51a2 100644 (file)
@@ -37,3 +37,14 @@ P Received: from CALLER by myhost.test.ex with local-smtp (Exim x.yz)
 I Message-Id: <E10HmbA-0005vi-00@myhost.test.ex>
 F From: x@y
   Date: Tue, 2 Mar 1999 09:44:33 +0000
+1999-03-02 09:44:33 10HmbB-0005vi-00 U=CALLER F=<> rejected after DATA: there is no valid sender in any header line
+Envelope-from: <>
+Envelope-to: <userx@test.ex>
+P Received: from CALLER by myhost.test.ex with local-smtp (Exim x.yz)
+       id 10HmbB-0005vi-00
+       for userx@test.ex; Tue, 2 Mar 1999 09:44:33 +0000
+T to: group name: x@y, p@q;
+R reply-to: group name:;
+I Message-Id: <E10HmbB-0005vi-00@myhost.test.ex>
+  Date: Tue, 2 Mar 1999 09:44:33 +0000
+  X-warning: this is a test warning
index e81e0d9d00d7a672b691a10078ff2920075a757a..7489cbd7470115680c78efa9b80a31005df983be 100644 (file)
@@ -104,4 +104,31 @@ Message 10
 .
 quit
 ****
+# Group syntax in reply-to header, but no address (falls back to From: for
+# header_sender check - From: is valid)
+exim -odi -bs
+mail from:<>
+rcpt to:<userx@test.ex>
+data
+to: group name: x@y, p@q;
+reply-to: group name:;
+from: userx@test.ex
+
+Message 11
+.
+quit
+****
+# Group syntax in reply-to header, but no address (falls back to From: for
+# header_sender check - but there is no From:)
+exim -odi -bs
+mail from:<>
+rcpt to:<userx@test.ex>
+data
+to: group name: x@y, p@q;
+reply-to: group name:;
+
+Message 12
+.
+quit
+****
 no_msglog_check
index 5020f18acc2a2c0a103d7d26b03d74eda6bdd8e2..7afb6b4d30b5eb8880a793c762f897423d876a18 100644 (file)
@@ -17,7 +17,7 @@
 >>> processing "require"
 >>> check verify = header_syntax
 >>> require: condition test failed
-LOG: 10HmbF-0005vi-00 H=[10.0.0.0] F=<x@y> rejected after DATA: domain missing or malformed: failing address in "From:" header is: @
+LOG: 10HmbH-0005vi-00 H=[10.0.0.0] F=<x@y> rejected after DATA: domain missing or malformed: failing address in "From:" header is: @
 >>> host in hosts_connection_nolog? no (option unset)
 >>> host in host_lookup? no (option unset)
 >>> host in host_reject_connection? no (option unset)
@@ -34,4 +34,4 @@ LOG: 10HmbF-0005vi-00 H=[10.0.0.0] F=<x@y> rejected after DATA: domain missing o
 >>> check condition = ${if match{$message_body}{trigger}{yes}{no}}
 >>>                 = yes
 >>> deny: condition test succeeded
-LOG: 10HmbG-0005vi-00 H=[10.0.0.0] F=<x@y> rejected after DATA: body contains trigger
+LOG: 10HmbI-0005vi-00 H=[10.0.0.0] F=<x@y> rejected after DATA: body contains trigger
index e02e9c7d33cd4e256395b59baeed80098fe24b45..f95e2ffb2217b111e2b357dd418b8e33ddbc5923 100644 (file)
@@ -2,7 +2,7 @@
 250 OK\r
 250 Accepted\r
 354 Enter message, ending with "." on a line by itself\r
-250 OK id=10HmbB-0005vi-00\r
+250 OK id=10HmbC-0005vi-00\r
 221 myhost.test.ex closing connection\r
 220 myhost.test.ex ESMTP Exim x.yz Tue, 2 Mar 1999 09:44:33 +0000\r
 250 OK\r
@@ -26,7 +26,7 @@
 250 OK\r
 250 Accepted\r
 354 Enter message, ending with "." on a line by itself\r
-250 OK id=10HmbC-0005vi-00\r
+250 OK id=10HmbD-0005vi-00\r
 221 myhost.test.ex closing connection\r
 220 myhost.test.ex ESMTP Exim x.yz Tue, 2 Mar 1999 09:44:33 +0000\r
 250 OK\r
@@ -38,7 +38,7 @@
 250 OK\r
 250 Accepted\r
 354 Enter message, ending with "." on a line by itself\r
-250 OK id=10HmbD-0005vi-00\r
+250 OK id=10HmbE-0005vi-00\r
 221 myhost.test.ex closing connection\r
 
 **** SMTP testing session as if from host 10.0.0.0
 250 OK\r
 250 Accepted\r
 354 Enter message, ending with "." on a line by itself\r
-250 OK id=10HmbE-0005vi-00\r
+250 OK id=10HmbF-0005vi-00\r
+221 myhost.test.ex closing connection\r
+220 myhost.test.ex ESMTP Exim x.yz Tue, 2 Mar 1999 09:44:33 +0000\r
+250 OK\r
+250 Accepted\r
+354 Enter message, ending with "." on a line by itself\r
+250 OK id=10HmbG-0005vi-00\r
+221 myhost.test.ex closing connection\r
+220 myhost.test.ex ESMTP Exim x.yz Tue, 2 Mar 1999 09:44:33 +0000\r
+250 OK\r
+250 Accepted\r
+354 Enter message, ending with "." on a line by itself\r
+550 Administrative prohibition\r
 221 myhost.test.ex closing connection\r