fetchmail-devel

[fetchmail-devel] [PATCH] imap trail broken

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

Hi,

Changes in imap_trail() in r4396 are incorrect and can cause segfault.
This patch should fix this.

===============================================================================
Index: fetchmail/imap.c
===================================================================
--- fetchmail/imap.c	(revision 4410)
+++ fetchmail/imap.c	(working copy)
@@ -1055,8 +1055,9 @@
 
 	/* UW IMAP returns "OK FETCH", Cyrus returns "OK Completed" */
 	if (strncmp(buf, tag, strlen(tag)) == 0) {
-	    t = buf + strspn(t, " \t");
-	    if (strncmp(t, "OK", 2))
+	    t = buf + strlen(tag);
+	    t += strspn(t, " \t");
+	    if (strncmp(t, "OK", 2) == 0)
 		break;
 	}
     }
===============================================================================

-- 
Sunil Shetye.

Re: [fetchmail-devel] [PATCH] imap trail broken

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

Sunil Shetye schrieb am 2005-11-11:

> Changes in imap_trail() in r4396 are incorrect and can cause segfault.

How can it cause segfault? It may eat too much garbage but I'd really
like to see the backtrace to investigate - if a malicious upstream
server can also trigger the segfault, we're in trouble.

> This patch should fix this.

Applied, thank you.

> 
> ===============================================================================
> Index: fetchmail/imap.c
> ===================================================================
> --- fetchmail/imap.c	(revision 4410)
> +++ fetchmail/imap.c	(working copy)
> @@ -1055,8 +1055,9 @@
>  
>  	/* UW IMAP returns "OK FETCH", Cyrus returns "OK Completed" */
>  	if (strncmp(buf, tag, strlen(tag)) == 0) {
> -	    t = buf + strspn(t, " \t");
> -	    if (strncmp(t, "OK", 2))
> +	    t = buf + strlen(tag);
> +	    t += strspn(t, " \t");
> +	    if (strncmp(t, "OK", 2) == 0)
>  		break;
>  	}
>      }
> ===============================================================================
> 

-- 
Matthias Andree

[fetchmail-devel] Re: imap trail broken

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

Quoting from Matthias Andree's mail on Mon, Nov 14, 2005 at 11:42:25PM +0100:
> > Changes in imap_trail() in r4396 are incorrect and can cause segfault.
> 
> How can it cause segfault? It may eat too much garbage but I'd really
> like to see the backtrace to investigate - if a malicious upstream
> server can also trigger the segfault, we're in trouble.

...

> > -	    t = buf + strspn(t, " \t");
                             ^

In the original code, the variable 't' passed to strspn() is
uninitialized.

Thomas Wolff has already reported a segfault for rc8 in the
fetchmail-users list. This could be a reason for that. Are you
releasing rc9 soon?

-- 
Sunil Shetye.

Re: [fetchmail-devel] Re: imap trail broken

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

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

> Quoting from Matthias Andree's mail on Mon, Nov 14, 2005 at 11:42:25PM +0100:
>> > Changes in imap_trail() in r4396 are incorrect and can cause segfault.
>> 
>> How can it cause segfault? It may eat too much garbage but I'd really
>> like to see the backtrace to investigate - if a malicious upstream
>> server can also trigger the segfault, we're in trouble.
>
> ...
>
>> > -	    t = buf + strspn(t, " \t");
>                              ^
>
> In the original code, the variable 't' passed to strspn() is
> uninitialized.

Urgh, right, it should have been buf as the first argument to strspn.
And it indeed forgot skipping over the tag.

> Thomas Wolff has already reported a segfault for rc8 in the
> fetchmail-users list. This could be a reason for that. Are you
> releasing rc9 soon?

Yes, really soon.

-- 
Matthias Andree