[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