Bug #89698 for MailTools: Miss-handling null "<>" email addresses

Tue Oct 22 11:34:58 2013 thorben.jaendling [...] switch.ch - Ticket created

Subject:	Miss-handling null "<>" email addresses
Date:	Tue, 22 Oct 2013 17:34:27 +0200
To:	bug-MailTools [...] rt.cpan.org, bug-AnyEvent-SMTP [...] rt.cpan.org
From:	Thorben Jändling <thorben.jaendling [...] switch.ch>

Good day, While trying to debug a SMTP conversation similar to the one that follows, I beleive that I have found a bug in Mail::Address (or in AnyEvent::SMTP's use of Mail::Address' parse() method): $ telnet my-mta 25 Trying x.x.x.x... Connected to my-mta. Escape character is '^]'. 220 my-mta AnyEvent::SMTP Ready. ehlo me 250 Go on. mail from:<> 501 Usage: MAIL FROM:<mail addr> quit 221 Bye. Connection to my-mta closed by foreign host. $ It is quite common, in the real world, to have no sender address. In such cases the null address "<>" is given. (Note "Mail from: \n" ie no <> is invalid and should error). In AnyEvent::SMTP::Server the 'mail from' handler has the following two lines: my @addrs = map { $_->address } Mail::Address->parse($from); @addrs == 1 or return $con->reply('501 Usage: MAIL FROM:<mail addr>'); However in the case of <> Mail::Address->parse returns an empty array; causing the SMTP conversation to incorrectly fail. Maybe AnyEvent::SMTP::Server should not be using this parse() method, or handle the <> case on its own. However my feeling is that parse should return a one element array, and the element should represent the null address (e.g. either "" or undef) Looking at Mail::Addess::parse(): It tokenises the string, but does not consider the case where there is no other token between < and >, ie: for(my $idx = 0; $idx < $len; $idx++) { $_ = $tokens->[$idx]; if(substr($_,0,1) eq '(') { push @comment, $_ } elsif($_ eq '<') { $depth++ } elsif($_ eq '>') { $depth-- if $depth } elsif($_ eq ',' || $_ eq ';') { ... } elsif($depth) { push @address, $_ } ... } The "elsif($depth)" will never be reached, thus the address array remains empty. I would suggest fixing the if/elsif logic or having _tokonise() add an element in the token array between < and >, maybe undef or ""; if there is no other token between them. Regards, Thorben -- Thorben Jändling - Security Engineer: +41 44 268 1576 SWITCH Security: cert@switch.ch +41 44 268 1540 http://www.switch.ch/all/cert/contact

Tue Oct 22 15:09:03 2013 Mark [...] Overmeer.net - Correspondence added

Subject:	Re: [rt.cpan.org #89698] Miss-handling null "<>" email addresses
Date:	Tue, 22 Oct 2013 21:08:34 +0200
To:	Thorben Jändling via RT <bug-MailTools [...] rt.cpan.org>
From:	Mark Overmeer <mark [...] overmeer.net>

* Thorben Jändling via RT (bug-MailTools@rt.cpan.org) [131022 15:35]: Show quoted text

> Tue Oct 22 11:34:58 2013: Request 89698 was acted upon. > Transaction: Ticket created by thorben.jaendling@switch.ch > Queue: MailTools > Subject: Miss-handling null "<>" email addresses > > It is quite common, in the real world, to have no sender address. In > such cases the null address "<>" is given. (Note "Mail from: \n" ie no > <> is invalid and should error).

Are you sure that it is common? I never receive emails without sender, and most spamfilters will block it. My full email collection (since 1993, >75k non-spam emails) does not contain any From or Sender with <>... It does contain a very small number of "Return-Path: <>", which is reasonable. Show quoted text

> In AnyEvent::SMTP::Server the 'mail from' handler has the following two > lines: > > my @addrs = map { $_->address } Mail::Address->parse($from); > @addrs == 1 or return $con->reply('501 Usage: MAIL FROM:<mail addr>'); > > However in the case of <> Mail::Address->parse returns an empty array; > causing the SMTP conversation to incorrectly fail.

When I see the code, I think the author wants to be sure that there is an sender address. <> is not an address, so the error seems justified. When I read rfc2822, I see angle-addr = [CFWS] "<" addr-spec ">" [CFWS] addr-spec = local-part "@" domain which makes an address <> illegal. -- MarkOv ------------------------------------------------------------------------ Mark Overmeer MSc MARKOV Solutions Mark@Overmeer.net solutions@overmeer.net http://Mark.Overmeer.net http://solutions.overmeer.net

Tue Oct 22 15:09:03 2013 The RT System itself - Status changed from 'new' to 'open'

Wed Oct 23 10:16:55 2013 thorben.jaendling [...] switch.ch - Correspondence added

Subject:	Re: [rt.cpan.org #89698] Miss-handling null "<>" email addresses
Date:	Wed, 23 Oct 2013 16:16:24 +0200
To:	bug-MailTools [...] rt.cpan.org
From:	Thorben Jändling <thorben.jaendling [...] switch.ch>

Hi Mark, Thank you for your reply. Show quoted text

>> It is quite common, in the real world, to have no sender address. In >> such cases the null address "<>" is given. (Note "Mail from: \n" ie no >> <> is invalid and should error).

> > Are you sure that it is common? I never receive emails without > sender, and most spamfilters will block it. > > My full email collection (since 1993, >75k non-spam emails) does not > contain any From or Sender with <>... It does contain a very small > number of "Return-Path: <>", which is reasonable.

We have a lot. Since we're also intentionally not filtering out spam before reaching the Perl application. About Return-Path and Sender, please see my comments below. Show quoted text

>> In AnyEvent::SMTP::Server the 'mail from' handler has the following two >> lines: >> >> my @addrs = map { $_->address } Mail::Address->parse($from); >> @addrs == 1 or return $con->reply('501 Usage: MAIL FROM:<mail addr>'); >> >> However in the case of <> Mail::Address->parse returns an empty array; >> causing the SMTP conversation to incorrectly fail.

> > When I see the code, I think the author wants to be sure that there > is an sender address. <> is not an address, so the error seems justified.

Not according to RFC 5321, which deals with SMTP: End of Section 3.6.3: " One way to prevent loops in error reporting is to specify a null reverse-path in the MAIL command of a notification message. When such a message is transmitted, the reverse-path MUST be set to null (see Section 4.5.5 for additional discussion). A MAIL command with a null reverse-path appears as follows: MAIL FROM:<> " In 4.1.1.2: " The reverse-path consists of the sender mailbox. .... , the reverse-path may be null.... " Section 4.1.2. defined the SMTP syntax, you'll see a Reverse-Path email address (and so MAIL FROM:) maybe <> Finally section 4.5.5. is all about such null reverse path email messages. Show quoted text

> When I read rfc2822, I see > angle-addr = [CFWS] "<" addr-spec ">" [CFWS] > addr-spec = local-part "@" domain > > which makes an address <> illegal. >

Well then 2822 and 5321 disagree with each other. In which case I'd opt for something that works for the "in the real world" case; where we both see <> addresses. I can accept the opinion that this all needs to be handled in the SMTP module and not your Mail::Address module. However it is still my feeling that Mail::Address should be able to acknowledge the existence of <> email addresses, so that it's user (e.g. the SMTP module) can act on it. I would suggest again that it returns "" (empty string) for <>. Otherwise a wrapping/using module would itself have to parse the entire address string to determine if any <> is in the address part and not in a comment or real-name part. i.e. could not just grep for /<>/ in an address string. Regards, Thorben -- Thorben Jändling - Security Engineer: +41 44 268 1576 SWITCH Security: cert@switch.ch +41 44 268 1540 http://www.switch.ch/all/cert/contact

Wed Oct 23 16:24:54 2013 solutions [...] overmeer.net - Correspondence added

Subject:	Re: [rt.cpan.org #89698] Miss-handling null "<>" email addresses
Date:	Wed, 23 Oct 2013 22:24:35 +0200
To:	Thorben Jändling via RT <bug-MailTools [...] rt.cpan.org>
From:	Mark Overmeer <solutions [...] overmeer.net>

* Thorben Jändling via RT (bug-MailTools@rt.cpan.org) [131023 14:17]: Show quoted text

> Queue: MailTools > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=89698 > > > Well then 2822 and 5321 disagree with each other. In which case I'd opt > for something that works for the "in the real world" case; where we both > see <> addresses.

2822 got followed-up by 5322. Also that only speaks about an empty return path. Show quoted text

> I can accept the opinion that this all needs to be handled in the SMTP > module and not your Mail::Address module. However it is still my feeling > that Mail::Address should be able to acknowledge the existence of <> > email addresses, so that it's user (e.g. the SMTP module) can act on it. > I would suggest again that it returns "" (empty string) for <>.

Apparently something has changed. Discussion is whether the STMP trick in the spec should be shown in the email address, because it is not an email address. The SMTP server can easily check for the path to be "<>" before parsing. I may go for changing Mail::Address, but will need to sleep another night on this subject. -- Regards, MarkOv ------------------------------------------------------------------------ Mark Overmeer MSc MARKOV Solutions Mark@Overmeer.net solutions@overmeer.net http://Mark.Overmeer.net http://solutions.overmeer.net

Mon Oct 28 08:16:52 2013 thorben.jaendling [...] switch.ch - Correspondence added

Subject:	Re: [rt.cpan.org #89698] Miss-handling null "<>" email addresses
Date:	Mon, 28 Oct 2013 13:16:28 +0100
To:	bug-MailTools [...] rt.cpan.org
From:	Thorben Jändling <thorben.jaendling [...] switch.ch>

Hi Mark, Just to let you know, my problem/bug has been fixed in the SMTP module, that uses Mail::Address, with the following changes: if ($from !~ /^\s*<>\s*$/) { @addrs = map { $_->address } Mail::Address->parse($from); @addrs == 1 or return $con->reply('501 Usage: MAIL FROM:<mail addr>'); } else { @addrs = (''); } and later (e.g. in RCPT handler): defined $con->{m}{from} or return $con->reply("503 Error: need MAIL command"); I am now able to receive and process such emails. At least with this SMTP module, that I'm using, I don't know about the others. You can close this ticket if you want, but I would be interested to know what you finally decide to do, if anything. Thank you and regards, Thorben On 23.10.13 22:24, Mark Overmeer via RT wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=89698 > > > * Thorben Jändling via RT (bug-MailTools@rt.cpan.org) [131023 14:17]:

>> Queue: MailTools >> Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=89698 > >> >> Well then 2822 and 5321 disagree with each other. In which case I'd opt >> for something that works for the "in the real world" case; where we both >> see <> addresses.

> > 2822 got followed-up by 5322. Also that only speaks about an > empty return path. >

>> I can accept the opinion that this all needs to be handled in the SMTP >> module and not your Mail::Address module. However it is still my feeling >> that Mail::Address should be able to acknowledge the existence of <> >> email addresses, so that it's user (e.g. the SMTP module) can act on it. >> I would suggest again that it returns "" (empty string) for <>.

> > Apparently something has changed. Discussion is whether the STMP > trick in the spec should be shown in the email address, because it is > not an email address. The SMTP server can easily check for the > path to be "<>" before parsing. > > I may go for changing Mail::Address, but will need to sleep another > night on this subject. >

-- Thorben Jändling - Security Engineer: +41 44 268 1576 SWITCH Security: cert@switch.ch +41 44 268 1540 http://www.switch.ch/all/cert/contact

Mon Oct 28 10:48:08 2013 solutions [...] overmeer.net - Correspondence added

Subject:	Re: [rt.cpan.org #89698] Miss-handling null "<>" email addresses
Date:	Mon, 28 Oct 2013 15:47:41 +0100
To:	Thorben Jändling via RT <bug-MailTools [...] rt.cpan.org>
From:	Mark Overmeer <solutions [...] overmeer.net>

* Thorben Jändling via RT (bug-MailTools@rt.cpan.org) [131028 12:17]: Show quoted text

> Queue: MailTools > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=89698 > > > if ($from !~ /^\s*<>\s*$/) { > @addrs = map { $_->address } Mail::Address->parse($from); > @addrs == 1 or return $con->reply('501 Usage: MAIL FROM:<mail addr>'); > } else { > @addrs = (''); > }

May I suggest: my @addrs; if ($from =~ /^\s*<>\s*$/) { { @addrs = (''); else { @addrs = map $_->address, Mail::Address->parse($from); @addrs == 1 or return $con->reply('501 Usage: MAIL FROM:<mail addr>'); } or (only 1 path through the code reduces bugs) my @addrs = $from =~ /^\s*<>\s*$/ ? '' : (map $_->address, Mail::Address->parse($from)); @addrs == 1 or return $con->reply('501 Usage: MAIL FROM:<mail addr>'); -- Regards, MarkOv ------------------------------------------------------------------------ Mark Overmeer MSc MARKOV Solutions Mark@Overmeer.net solutions@overmeer.net http://Mark.Overmeer.net http://solutions.overmeer.net

Fri Oct 10 14:18:58 2014 MARKOV [...] cpan.org - Correspondence added

not added to MailTools

Fri Oct 10 14:18:59 2014 MARKOV [...] cpan.org - Status changed from 'open' to 'resolved'