Re: Managesieve segfault bug. Test case and work around included

On Tue, Jul 19, 2011 at 10:22:05PM -0700, Steven Kurylo wrote:
> Hi,
> 
> I've tried perdition_1.19~rc4-3 but I still get a segfault.  I'm
> running cyrus 2.4.8, and I can avoid the segfault under two scenarios:
> 1) With "sasl_mech_list: LOGIN PLAIN" and "allowplaintext: no" in my
> cyrus config
> 2) With "sasl_mech_list: PLAIN" and "allowplaintext: yes" in my cyrus config
> 
> In my environment, we have a client which can't do TLS, so a special
> sieve daemon which allows plaintext listens on local host for it.
> Originally #2 also offered LOGIN and I that's how I ran into the
> segfault.
> 
> It seems to be something in the sieve banner parsing.  I've made a
> python script to act as a sieve server (see attached):
> ./server.py
> 
> Then I run perdition:
> perdition.managesieve  -d --no_daemon --listen_port=2006 -C
> --outgoing_server=127.0.0.1:2005 -f '' --pid_file /tmp/sieve.pid
> 
> Then I telnet to 2006 and paste :
> AUTHENTICATE "PLAIN" "<real auth string>"
> 
> I either get an OK or a segfault in perdition.  server.py will cycle
> through the specified banners, so you can continue to telnet to try
> each banner test case out.
> 
> Let me know if you need any further information.  Thanks.

Hi,

sorry for taking an extraordinary long time to respond to this.

Using your test case I am able to reproduce the problem and
I believe that it is resolved by the following change
which I have pushed into the mercurial tree. If someone could
verify the change that would be great.

# HG changeset patch
# User Simon Horman <[email protected]>
# Date 1313971959 -32400
# Node ID 2b24fd4c0978098e42d23d578431fa8292336c1d
# Parent  6544c58ec1445f85276f1128821b5d2ceeb64594
managesieve: Fix handling "PLAIN LOGIN"

This corrects a logic bug which resulted in a segmentation fault
in strcasestr() if the needle did not match the beginning of the haystack.

diff -r 6544c58ec144 -r 2b24fd4c0978 perdition/str.c
--- a/perdition/str.c	Tue Dec 21 07:44:19 2010 +0900
+++ b/perdition/str.c	Mon Aug 22 09:12:39 2011 +0900
@@ -728,7 +728,7 @@
 	haystack_len = strlen(haystack);
 	needle_len = strlen(needle);
 
-	for (i = 0; haystack_len + i >= needle_len; i++)
+	for (i = 0; haystack_len - i >= needle_len; i++)
 		if (!strcasecmp(haystack + i, needle))
 			return haystack + i;
 
______________________________________________
Perdition-users mailing list
[email protected]
http://lists.vergenet.net/listinfo/perdition-users