fetchmail-devel

[fetchmail-devel] Re: fetchmail segfault on 'empty' mail

[Matthias A. <mat...@gm...>]

[resending to the list, too, with more information on what I've tried to
reproduce]

Please take everything to the berlios.de lists (you'll need to
subscribe there) - we plan to leave the ccil.org lists since nobody has
administrative access to them any more.

On Mon, 05 Dec 2005, Daniel Drake wrote:

> Hi,
> 
> I noticed that my fetchmail was segfaulting at the very start of a 
> particular mail message, with this trace:
> 
> #0  0xb7e67423 in strlen () from /lib/tls/libc.so.6
> #1  0x0805cded in readheaders (sock=6, fetchlen=0, reallen=0, 
> ctl=0x808d2d8, num=2, suppress_readbody=0xbff7c835 "")
>     at transact.c:920
> #2  0x080597df in fetch_messages (mailserver_socket=6, ctl=0x808d2d8, 
> count=272, msgsizes=0xbff7c7f0, maxfetch=0,
>     fetches=0xbff7e8c0, dispatches=0xbff7e8bc, deletions=0xbff7e8cc) at 
> driver.c:614
> #3  0x0805ae82 in do_session (ctl=0x808d2d8, proto=0x8071da0, maxfetch=0) 
> at driver.c:1449
> #4  0x0805b39d in do_protocol (ctl=0x808d2d8, proto=0x8071da0) at 
> driver.c:1622
> #5  0x0804f81a in doPOP3 (ctl=0x808d2d8) at pop3.c:1215
> #6  0x08054c11 in query_host (ctl=0x808d2d8) at fetchmail.c:1373
> #7  0x08052c26 in main (argc=4, argv=0xbff80bd4) at fetchmail.c:646
> 
> It is downloading mail from POP3.

I seem to be unable to reproduce this. I've tried the POP3 modules of
Dovecot 0.99.14 and some older Courier-IMAP.

What upstream server software are you using,
who built your fetchmail version,
and what is your .fetchmailrc?

> --- fetchmail-6.2.5/transact.c.orig	2005-12-05 15:25:54.000000000 +0000
> +++ fetchmail-6.2.5/transact.c	2005-12-05 16:16:45.000000000 +0000
> @@ -511,7 +511,7 @@ int readheaders(int sock,
>  	    }
>  
>  	    /* check for end of headers */
> -	    if (end_of_header(line))
> +	    if (msgblk.headers && end_of_header(line))
>  	    {
>  		if (linelen != strlen (line))
>  		    has_nuls = TRUE;

What has "msgblk.headers" got to do with "line"? I'm not particular fond
of such skewed checks, they'll come back some day to bite us.

-- 
Matthias Andree

Re: [fetchmail-devel] [PATCH] Re: fetchmail segfault on 'empty' mail

[Daniel D. <ds...@ge...>]

Sunil Shetye wrote:
> The problem is specific to multidrop mailbox only where the
> duplicate-message killer cannot handle a headerless mail. The attached
> patch should fix this issue. 
> 
> The patch also cleans up the code by calling free() at one point
> only for static variables.

Thanks.

I haven't looked at the freeing changes, but I can confirm that this hunk 
alone would have fixed the problem:


> @@ -936,7 +928,7 @@
>       * to break it in a way that blackholed mail.  Better to pass
>       * the occasional duplicate than to do that...
>       */
> -    if (MULTIDROP(ctl))
> +    if (MULTIDROP(ctl) && msgblk.headers)
>      {
>  	MD5_CTX context;
>  

I also think it probably makes more sense than my change did. Would be great 
to see it included in the next versions of fetchmail :)

Thanks,
Daniel

Re: [fetchmail-devel] [PATCH] Re: fetchmail segfault on 'empty' mail

[Matthias A. <mat...@gm...>]

Daniel Drake <ds...@ge...> writes:

>> @@ -936,7 +928,7 @@
>>       * to break it in a way that blackholed mail.  Better to pass
>>       * the occasional duplicate than to do that...
>>       */
>> -    if (MULTIDROP(ctl))
>> +    if (MULTIDROP(ctl) && msgblk.headers)
>>      {
>>  	MD5_CTX context;
>>
>
> I also think it probably makes more sense than my change did. Would be great 
> to see it included in the next versions of fetchmail :)

Thanks for testing it. The change is already in the SVN trunk and
BRANCH_6-3, so expect to see it in fetchmail-6.3.1 somewhen next week.

-- 
Matthias Andree

[fetchmail-devel] Re: fetchmail segfault on 'empty' mail

[Sunil S. <sh...@bo...>]

Hi Daniel,

Quoting from Daniel Drake's mail on Thu, Dec 08, 2005:
> Thanks.
> 
> I haven't looked at the freeing changes, but I can confirm that this hunk 
> alone would have fixed the problem:

...

> I also think it probably makes more sense than my change did. Would be 
> great to see it included in the next versions of fetchmail :)

Thanks for following up on this issue.

On a side note, I feel that the empty messages point to a problem of
mail delivery error on your ISP side. For even if somebody sends you
an empty mail, your ISP must be adding atleast two headers
(Return-Path: and Delivered-To:). Otherwise, your multidrop setup
would not have worked at all. So, even a minimal mail cannot be
headerless.

-- 
Sunil Shetye.

Re: [fetchmail-devel] Re: fetchmail segfault on 'empty' mail

[Matthias A. <mat...@gm...>]

Sunil Shetye <sh...@bo...> writes:

> On a side note, I feel that the empty messages point to a problem of
> mail delivery error on your ISP side. For even if somebody sends you
> an empty mail, your ISP must be adding atleast two headers
> (Return-Path: and Delivered-To:). Otherwise, your multidrop setup
> would not have worked at all. So, even a minimal mail cannot be
> headerless.

It is also a violation of the 23 year old RFC-822 which requires that at
least From: and Date: and either of To: or Bcc: must be present.

-- 
Matthias Andree

[fetchmail-devel] Re: [fetchmail]Re: fetchmail segfault on 'empty' mail

[Daniel D. <ds...@ge...>]

Matthias Andree wrote:
> On Mon, 05 Dec 2005, Daniel Drake wrote:
> 
> 
>>Hi,
>>
>>I noticed that my fetchmail was segfaulting at the very start of a 
>>particular mail message, with this trace:
>>
>>#0  0xb7e67423 in strlen () from /lib/tls/libc.so.6
>>#1  0x0805cded in readheaders (sock=6, fetchlen=0, reallen=0, 
>>ctl=0x808d2d8, num=2, suppress_readbody=0xbff7c835 "")
>>    at transact.c:920
>>#2  0x080597df in fetch_messages (mailserver_socket=6, ctl=0x808d2d8, 
>>count=272, msgsizes=0xbff7c7f0, maxfetch=0,
>>    fetches=0xbff7e8c0, dispatches=0xbff7e8bc, deletions=0xbff7e8cc) at 
>>driver.c:614
>>#3  0x0805ae82 in do_session (ctl=0x808d2d8, proto=0x8071da0, maxfetch=0) 
>>at driver.c:1449
>>#4  0x0805b39d in do_protocol (ctl=0x808d2d8, proto=0x8071da0) at 
>>driver.c:1622
>>#5  0x0804f81a in doPOP3 (ctl=0x808d2d8) at pop3.c:1215
>>#6  0x08054c11 in query_host (ctl=0x808d2d8) at fetchmail.c:1373
>>#7  0x08052c26 in main (argc=4, argv=0xbff80bd4) at fetchmail.c:646
>>
>>It is downloading mail from POP3.
> 
> 
> I seem to be unable to reproduce this. I've tried the POP3 modules of
> Dovecot 0.99.14 and some older Courier-IMAP.
> 
> What upstream server software are you using,
> who built your fetchmail version,
> and what is your .fetchmailrc?

I don't know what the upstream server runs. Is there a way of figuring this 
out through the server? It is an ISP POP3 server running at 
aap.businessserve.co.uk

I am running Gentoo Linux which compiles fetchmail from source, however I also 
reproduced the bug on an unmodified locally compiled fetchmail-6.3.0

My .fetchmailrc :

poll aap.businessserve.co.uk proto pop3 no dns localdomains mydomain.com:
envelope "Delivered-To:"
user xxx with pass xxx to * here keep

>>--- fetchmail-6.2.5/transact.c.orig	2005-12-05 15:25:54.000000000 +0000
>>+++ fetchmail-6.2.5/transact.c	2005-12-05 16:16:45.000000000 +0000
>>@@ -511,7 +511,7 @@ int readheaders(int sock,
>> 	    }
>> 
>> 	    /* check for end of headers */
>>-	    if (end_of_header(line))
>>+	    if (msgblk.headers && end_of_header(line))
>> 	    {
>> 		if (linelen != strlen (line))
>> 		    has_nuls = TRUE;
> 
> 
> What has "msgblk.headers" got to do with "line"? I'm not particular fond
> of such skewed checks, they'll come back some day to bite us.
> 

Ok, I'll try and explain better:

readheaders() is called.

readheaders() nulls out msgblk.headers and starts processing the mail.

The first line read is just a literal '\r\n', so we have line="\r\n" and 
linelen=2. We skip a couple of checks (line does not exceed MSGBUFSIZE, line 
is properly CRLF terminated), and we get down to line 520:

	    /* check for end of headers */
	    if (end_of_header(line))
	    {
		if (linelen != strlen (line))
		    has_nuls = TRUE;
		free(line);
		goto process_headers;
	    }

end_of_headers(line) succeeds - line *is* just a literal \r\n so it does look 
like the blank line inbetween headers and body. So we now jump to process_headers.

[ I just noticed that the crash occurs in a MULTIDROP-only section of code. 
Maybe this is why you can't reproduce it. ]

Working down the function from process_headers, we get down to line 953:

	MD5Update(&context, msgblk.headers, strlen(msgblk.headers));

As we haven't touched msgblk.headers since earlier, it is still NULL, so we 
call strlen(NULL) which causes a segfault.

Of course, there are other ways of circumventing this problem. I think my 
patch does have *some* sensible logic behind it - i.e. check that we do 
actually have headers before trying to figure out if the headers are ending, 
but I'll leave it to you if you can think of more appropriate ways.

Thanks,
Daniel

[fetchmail-devel] Re: fetchmail segfault on 'empty' mail

[Sunil S. <sh...@bo...>]

Quoting from Daniel Drake's mail on Tue, Dec 06, 2005:
> I don't know what the upstream server runs. Is there a way of figuring this 
> out through the server? It is an ISP POP3 server running at 
> aap.businessserve.co.uk
> 
> I am running Gentoo Linux which compiles fetchmail from source, however I 
> also reproduced the bug on an unmodified locally compiled fetchmail-6.3.0
> 
> My .fetchmailrc :
> 
> poll aap.businessserve.co.uk proto pop3 no dns localdomains mydomain.com:
> envelope "Delivered-To:"
> user xxx with pass xxx to * here keep

Please send the output of "fetchmail -v" before and after the patch
when you have an empty mail in your mailbox. This will show that the
patch indeed works correctly and only that empty mail is getting
properly deleted when you have multiple mails in your mailbox.

Do you keep on getting such empty mails frequently? Or, did that
happen only once?

-- 
Sunil Shetye.

[fetchmail-devel] Re: fetchmail segfault on 'empty' mail

[Daniel D. <ds...@ge...>]

Hi Sunil,

Sunil Shetye wrote:
> Please send the output of "fetchmail -v" before and after the patch
> when you have an empty mail in your mailbox. This will show that the
> patch indeed works correctly and only that empty mail is getting
> properly deleted when you have multiple mails in your mailbox.

I don't have any more of these mails right now, so this is fairly fabricated, 
but more or less accurate.

Before:

fetchmail: 6.2.5.2 querying aap.businessserve.co.uk (protocol POP3) at Wed, 07 
Dec 2005 13:20:02 +0000 (GMT): poll started
fetchmail: POP3< +OK <..>
fetchmail: POP3> CAPA
fetchmail: POP3< -ERR authorization first
fetchmail: authorization first
fetchmail: Repoll immediately on xxx
fetchmail: POP3< +OK <..>
fetchmail: POP3> USER xxx
fetchmail: POP3< +OK
fetchmail: POP3> PASS *
fetchmail: POP3< +OK
fetchmail: POP3> STAT
fetchmail: POP3< +OK 26 82969
fetchmail: POP3> LIST 1
fetchmail: POP3< +OK 1 1329
fetchmail: POP3> RETR 1
fetchmail: POP3< +OK 1329 octets follow.
reading message xyz:1 of 26 (1329 octets)
Segmentation fault

After:

fetchmail: 6.2.5.2 querying aap.businessserve.co.uk (protocol POP3) at Wed, 07 
Dec 2005 13:20:02 +0000 (GMT): poll started
fetchmail: POP3< +OK <..>
fetchmail: POP3> CAPA
fetchmail: POP3< -ERR authorization first
fetchmail: authorization first
fetchmail: Repoll immediately on xxx
fetchmail: POP3< +OK <..>
fetchmail: POP3> USER xxx
fetchmail: POP3< +OK
fetchmail: POP3> PASS *
fetchmail: POP3< +OK
fetchmail: POP3> STAT
fetchmail: POP3< +OK 26 82969
fetchmail: POP3> LIST 1
fetchmail: POP3< +OK 1 1329
fetchmail: POP3> RETR 1
fetchmail: POP3< +OK 1329 octets follow.
reading message xyz:1 of 26 (1329 octets)
***** message delimiter found while scanning headers
flushed
fetchmail: POP3> LIST 2
...

> Do you keep on getting such empty mails frequently? Or, did that
> happen only once?

This system has been running fetchmail for about a year, and this is the first 
time I have seen this. 5 of these mails were received in a short space of time.

Daniel

[fetchmail-devel] [PATCH] Re: fetchmail segfault on 'empty' mail

[Sunil S. <sh...@bo...>]

Hi,

Quoting from Daniel Drake's mail on Wed, Dec 07, 2005:
> I don't have any more of these mails right now, so this is fairly 
> fabricated, but more or less accurate.
> 
> Before:
> 
> fetchmail: 6.2.5.2 querying aap.businessserve.co.uk (protocol POP3) at Wed, 
> 07 Dec 2005 13:20:02 +0000 (GMT): poll started
> fetchmail: POP3< +OK <..>
> fetchmail: POP3> CAPA
> fetchmail: POP3< -ERR authorization first
> fetchmail: authorization first
> fetchmail: Repoll immediately on xxx
> fetchmail: POP3< +OK <..>
> fetchmail: POP3> USER xxx
> fetchmail: POP3< +OK
> fetchmail: POP3> PASS *
> fetchmail: POP3< +OK
> fetchmail: POP3> STAT
> fetchmail: POP3< +OK 26 82969
> fetchmail: POP3> LIST 1
> fetchmail: POP3< +OK 1 1329
> fetchmail: POP3> RETR 1
> fetchmail: POP3< +OK 1329 octets follow.
> reading message xyz:1 of 26 (1329 octets)
> Segmentation fault

The problem is specific to multidrop mailbox only where the
duplicate-message killer cannot handle a headerless mail. The attached
patch should fix this issue. 

The patch also cleans up the code by calling free() at one point
only for static variables.

-- 
Sunil Shetye.

Re: [fetchmail-devel] [PATCH] Re: fetchmail segfault on 'empty' mail

[Matthias A. <mat...@gm...>]

Sunil Shetye <sh...@bo...> writes:

> The problem is specific to multidrop mailbox only where the
> duplicate-message killer cannot handle a headerless mail. The attached
> patch should fix this issue. 
>
> The patch also cleans up the code by calling free() at one point
> only for static variables.

I've merged it with trivial changes (I've left the braces in because the
patches are easier to review this way and ambiguities are avoided).

I cannot test the new code (no multidrop here, let alone broken
multidrop that needs to parse headers), I hope it doesn't break
anything, and I'm looking forward to hear from Daniel and others who use
multidrop configurations if this patch works OK for them.

It appears as though all regression and new bug reports had been
addressed and we might see 6.3.1 soon.

-- 
Matthias Andree