From 1eccaa59eb366c180c36af219a142d8f934f73b0 Mon Sep 17 00:00:00 2001 From: Philip Hazel Date: Tue, 10 Oct 2006 15:36:50 +0000 Subject: [PATCH] Sort out group syntax problems, particularly with verify=header_sender. --- doc/doc-txt/ChangeLog | 13 +++++- src/src/parse.c | 15 ++++-- src/src/receive.c | 13 +++--- src/src/sieve.c | 4 +- src/src/verify.c | 88 +++++++++++++++++++++++++----------- test/log/0026 | 14 ++++-- test/mail/0026.userx | 21 +++++++-- test/rejectlog/0026 | 11 +++++ test/scripts/0000-Basic/0026 | 27 +++++++++++ test/stderr/0026 | 4 +- test/stdout/0026 | 20 ++++++-- 11 files changed, 175 insertions(+), 55 deletions(-) diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog index 6d25eec70..e09227ded 100644 --- a/doc/doc-txt/ChangeLog +++ b/doc/doc-txt/ChangeLog @@ -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 ----------------- diff --git a/src/src/parse.c b/src/src/parse.c index a0366431e..0c1b9fe8f 100644 --- a/src/src/parse.c +++ b/src/src/parse.c @@ -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 diff --git a/src/src/receive.c b/src/src/receive.c index 797444ca1..73675bee2 100644 --- a/src/src/receive.c +++ b/src/src/receive.c @@ -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 diff --git a/src/src/sieve.c b/src/src/sieve.c index 425a0b9e0..549dba197 100644 --- a/src/src/sieve.c +++ b/src/src/sieve.c @@ -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; diff --git a/src/src/verify.c b/src/src/verify.c index 4f0da9bbd..5b635989e 100644 --- a/src/src/verify.c +++ b/src/src/verify.c @@ -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"; diff --git a/test/log/0026 b/test/log/0026 index be92f738a..cf9958438 100644 --- a/test/log/0026 +++ b/test/log/0026 @@ -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= 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= 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 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 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 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 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 diff --git a/test/mail/0026.userx b/test/mail/0026.userx index 74f68e2cb..4c7822197 100644 --- a/test/mail/0026.userx +++ b/test/mail/0026.userx @@ -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 ) - id 10HmbD-0005vi-00 + id 10HmbE-0005vi-00 for userx@test.ex; Tue, 2 Mar 1999 09:44:33 +0000 -Message-Id: +Message-Id: 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 ) - 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: +Message-Id: 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: +Date: Tue, 2 Mar 1999 09:44:33 +0000 +X-warning: this is a test warning + +Message 11 + diff --git a/test/rejectlog/0026 b/test/rejectlog/0026 index 384d6ef6e..716e35cf0 100644 --- a/test/rejectlog/0026 +++ b/test/rejectlog/0026 @@ -37,3 +37,14 @@ P Received: from CALLER by myhost.test.ex with local-smtp (Exim x.yz) I Message-Id: 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: +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: + Date: Tue, 2 Mar 1999 09:44:33 +0000 + X-warning: this is a test warning diff --git a/test/scripts/0000-Basic/0026 b/test/scripts/0000-Basic/0026 index e81e0d9d0..7489cbd74 100644 --- a/test/scripts/0000-Basic/0026 +++ b/test/scripts/0000-Basic/0026 @@ -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: +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: +data +to: group name: x@y, p@q; +reply-to: group name:; + +Message 12 +. +quit +**** no_msglog_check diff --git a/test/stderr/0026 b/test/stderr/0026 index 5020f18ac..7afb6b4d3 100644 --- a/test/stderr/0026 +++ b/test/stderr/0026 @@ -17,7 +17,7 @@ >>> processing "require" >>> check verify = header_syntax >>> require: condition test failed -LOG: 10HmbF-0005vi-00 H=[10.0.0.0] F= rejected after DATA: domain missing or malformed: failing address in "From:" header is: @ +LOG: 10HmbH-0005vi-00 H=[10.0.0.0] F= 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= 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= rejected after DATA: body contains trigger +LOG: 10HmbI-0005vi-00 H=[10.0.0.0] F= rejected after DATA: body contains trigger diff --git a/test/stdout/0026 b/test/stdout/0026 index e02e9c7d3..f95e2ffb2 100644 --- a/test/stdout/0026 +++ b/test/stdout/0026 @@ -2,7 +2,7 @@ 250 OK 250 Accepted 354 Enter message, ending with "." on a line by itself -250 OK id=10HmbB-0005vi-00 +250 OK id=10HmbC-0005vi-00 221 myhost.test.ex closing connection 220 myhost.test.ex ESMTP Exim x.yz Tue, 2 Mar 1999 09:44:33 +0000 250 OK @@ -26,7 +26,7 @@ 250 OK 250 Accepted 354 Enter message, ending with "." on a line by itself -250 OK id=10HmbC-0005vi-00 +250 OK id=10HmbD-0005vi-00 221 myhost.test.ex closing connection 220 myhost.test.ex ESMTP Exim x.yz Tue, 2 Mar 1999 09:44:33 +0000 250 OK @@ -38,7 +38,7 @@ 250 OK 250 Accepted 354 Enter message, ending with "." on a line by itself -250 OK id=10HmbD-0005vi-00 +250 OK id=10HmbE-0005vi-00 221 myhost.test.ex closing connection **** SMTP testing session as if from host 10.0.0.0 @@ -66,5 +66,17 @@ 250 OK 250 Accepted 354 Enter message, ending with "." on a line by itself -250 OK id=10HmbE-0005vi-00 +250 OK id=10HmbF-0005vi-00 +221 myhost.test.ex closing connection +220 myhost.test.ex ESMTP Exim x.yz Tue, 2 Mar 1999 09:44:33 +0000 +250 OK +250 Accepted +354 Enter message, ending with "." on a line by itself +250 OK id=10HmbG-0005vi-00 +221 myhost.test.ex closing connection +220 myhost.test.ex ESMTP Exim x.yz Tue, 2 Mar 1999 09:44:33 +0000 +250 OK +250 Accepted +354 Enter message, ending with "." on a line by itself +550 Administrative prohibition 221 myhost.test.ex closing connection -- 2.25.1