Bug#408174: exim4: header_sender test and group addresses: exim rejecting rfc-valid mails

Marc Haber mh+debian-packages at zugschlus.de
Fri Jan 26 16:41:11 UTC 2007


On Fri, Jan 26, 2007 at 05:39:28PM +0100, Marc Haber wrote:
> Magnus Holmgren kindly pulled the attached patch

And now I have actually attached it.

Greetings
Marc

-- 
-----------------------------------------------------------------------------
Marc Haber         | "I don't trust Computers. They | Mailadresse im Header
Mannheim, Germany  |  lose things."    Winona Ryder | Fon: *49 621 72739834
Nordisch by Nature |  How to make an American Quilt | Fax: *49 621 72739835
-------------- next part --------------
#! /bin/sh /usr/share/dpatch/dpatch-run
## 80_upstream_408174_4-64-PH18.dpatch by Marc Haber <mh+debian-packages at zugschlus.de>
##
## All lines beginning with `## DP:' are a description of the patch.
## DP: No description.

@DPATCH@
diff -urNad trunk~/src/parse.c trunk/src/parse.c
--- trunk~/src/parse.c	2006-07-31 16:19:48.000000000 +0200
+++ trunk/src/parse.c	2007-01-26 17:15:16.000000000 +0100
@@ -1,4 +1,4 @@
-/* $Cambridge: exim/exim-src/src/parse.c,v 1.9 2006/03/08 11:13:07 ph10 Exp $ */
+/* $Cambridge: exim/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 @@
 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
diff -urNad trunk~/src/receive.c trunk/src/receive.c
--- trunk~/src/receive.c	2006-07-31 16:19:48.000000000 +0200
+++ trunk/src/receive.c	2007-01-26 17:15:16.000000000 +0100
@@ -2051,8 +2051,6 @@
     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 @@
       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 @@
 
         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 @@
       }   /* 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
diff -urNad trunk~/src/sieve.c trunk/src/sieve.c
--- trunk~/src/sieve.c	2006-07-31 16:19:48.000000000 +0200
+++ trunk/src/sieve.c	2007-01-26 17:15:16.000000000 +0100
@@ -1826,6 +1826,8 @@
         if (saveend == 0) break;
         header_value = end_addr + 1;
         }
+      parse_allow_group = FALSE;
+      parse_found_group = FALSE;
       }
     }
   return 1;
diff -urNad trunk~/src/verify.c trunk/src/verify.c
--- trunk~/src/verify.c	2006-07-31 16:19:48.000000000 +0200
+++ trunk/src/verify.c	2007-01-26 17:15:16.000000000 +0100
@@ -1376,8 +1376,9 @@
 {
 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 &&
@@ -1391,9 +1392,10 @@
   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)
     {
@@ -1403,7 +1405,7 @@
     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);
@@ -1459,7 +1461,8 @@
         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 */
@@ -1467,9 +1470,12 @@
     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;
 }
 
 
@@ -1512,9 +1518,10 @@
     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)
       {
@@ -1524,7 +1531,7 @@
       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);
@@ -1548,6 +1555,9 @@
       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;
@@ -1630,13 +1640,14 @@
   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;
@@ -1644,6 +1655,11 @@
     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;
@@ -1686,11 +1702,21 @@
       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. */
@@ -1698,14 +1724,13 @@
         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*
@@ -1739,15 +1764,24 @@
 
       /* 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";


More information about the Pkg-exim4-maintainers mailing list