Skip Menu |
 

This queue is for tickets about the AnyEvent-SMTP CPAN distribution.

Report information
The Basics
Id: 89699
Status: open
Priority: 0/
Queue: AnyEvent-SMTP

People
Owner: MONS [...] cpan.org
Requestors: thorben.jaendling [...] switch.ch
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: (no value)



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
Subject: Re: [rt.cpan.org #89699] AutoReply: Miss-handling null "<>" email addresses
Date: Wed, 23 Oct 2013 16:19:08 +0200
To: bug-AnyEvent-SMTP [...] rt.cpan.org
From: Thorben Jändling <thorben.jaendling [...] switch.ch>
Download (untitled) / with headers
text/plain 3.2k
Hi, I simultaneously posted this bug to the Mail::Address module bug queue, as it was not clear where the problem is. The other ticket is #89698 and now has more details about the problem, including a discussion on RFC specs: https://rt.cpan.org/Public/Bug/Display.html?id=89698 Regards, Thorben On 22.10.13 17:34, Bugs in AnyEvent-SMTP via RT wrote: Show quoted text
> > Greetings, > > This message has been automatically generated in response to the > creation of a trouble ticket regarding: > "Miss-handling null "<>" email addresses", > a summary of which appears below. > > There is no need to reply to this message right now. Your ticket has been > assigned an ID of [rt.cpan.org #89699]. Your ticket is accessible > on the web at: > > https://rt.cpan.org/Ticket/Display.html?id=89699 > > Please include the string: > > [rt.cpan.org #89699] > > in the subject line of all future correspondence about this issue. To do so, > you may reply to this message. > > Thank you, > bug-AnyEvent-SMTP@rt.cpan.org > > ------------------------------------------------------------------------- > 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
Thank you for the detailed report. Fixes commited and will appear soon on CPAN with v0.09
Subject: Re: [rt.cpan.org #89699] Miss-handling null "<>" email addresses
Date: Fri, 25 Oct 2013 11:10:00 +0200
To: bug-AnyEvent-SMTP [...] rt.cpan.org
From: Thorben Jändling <thorben.jaendling [...] switch.ch>
Hi Mons, Thank you for acting on this. However your fix is not complete: In other part of your code (e.g. the RCPT and DATA handlers) you have tests like this: $con->{m}{from} or return $con->reply("503 Error: need MAIL command"); If $con->{m}{from} is "" it will be equated to false. You probably want to test for defined. e.g. defined( $con->{m}{from} ) or return $con->reply("503 Error: need MAIL command"); Otherwise the SMTP sessions just incorrectly fails later on: $ nc localhost 2525 220 perl AnyEvent::SMTP Ready. ehlo localhost-test 250 Go on. mail from:<> 250 Ok. RCPT TO:test@localhost 503 Error: need MAIL command bye 500 Learn to type! quit 221 Bye. Regards, Thorben On 24.10.13 23:00, Mons Anderson via RT wrote: Show quoted text
> <URL: https://rt.cpan.org/Ticket/Display.html?id=89699 > > > Thank you for the detailed report. > Fixes commited and will appear soon on CPAN with v0.09 >
-- 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
Download (untitled) / with headers
text/plain 146b
I was expecting my reply from my work address to re-open this, but it didn't. See last email, the fix only moved the bug to the next SMTP command.
Download (untitled) / with headers
text/plain 324b
On Fri Oct 25 07:08:38 2013, TREVELYAN wrote: Show quoted text
> I was expecting my reply from my work address to re-open this, but it > didn't. > See last email, the fix only moved the bug to the next SMTP command.
I've commited it here: https://github.com/Mons/AnyEvent-SMTP Please, make your checks. If everything is ok, I'll upload it.
Subject: Re: [rt.cpan.org #89699] Miss-handling null "<>" email addresses
Date: Fri, 25 Oct 2013 13:40:50 +0200
To: bug-AnyEvent-SMTP [...] rt.cpan.org
From: Thorben Jändling <thorben.jaendling [...] switch.ch>
Download (untitled) / with headers
text/plain 925b
Hi, Looks fine: $ nc localhost 2525 220 perl AnyEvent::SMTP Ready. ehlo test 250 Go on. mail from:<> 250 Ok. rcpt to:text@localhost 250 Ok. data 354 End data with <CR><LF>.<CR><LF> From: test To: test@localhost Subject: test test 1 2 3 . 250 I'll take it quit 221 Bye. Thanks for being so prompt. Regards, Thorben On 25.10.13 13:24, Mons Anderson via RT wrote: Show quoted text
> <URL: https://rt.cpan.org/Ticket/Display.html?id=89699 > > > On Fri Oct 25 07:08:38 2013, TREVELYAN wrote:
>> I was expecting my reply from my work address to re-open this, but it >> didn't. >> See last email, the fix only moved the bug to the next SMTP command.
> > I've commited it here: > https://github.com/Mons/AnyEvent-SMTP > > Please, make your checks. > If everything is ok, I'll upload it. >
-- 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


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.