dovecot - Sep 2011 - [PATCH] Bad boundary check in client_find_namespace

Hi,

while trying to investigate the bug I reported last week, I found that
there is a broken boundary check in client_find_namespace in
src/imap/imap-commands-util.c.  The code is:

    /* make sure two hierarchy separators aren't next to each others */
    for (p = storage_name+1; *p != '\0'; p++) {
    	if (p[0] == ns->real_sep && p[-1] == ns->real_sep) {
    		client_send_tagline(cmd, "NO Invalid mailbox name.");
    		return NULL;
    	}
    }

The loop iterates over the string storage_name starting from its second
byte.  However, the string may be "".  This is the case if you select
the root of a namespace, like in "SELECT shared" or
"SELECT shared/user1".  In that case, the code will read past the end
of
the buffer from random memory until it finds a zero byte or a duplicate
separator.

This would fix it:

diff -ru dovecot-2.0.15.orig/src/imap/imap-commands-util.c
dovecot-2.0.15/src/imap/imap-commands-util.c
--- dovecot-2.0.15.orig/src/imap/imap-commands-util.c	2011-08-02
12:29:37.000000000 +0200
+++ dovecot-2.0.15/src/imap/imap-commands-util.c	2011-09-26 18:33:16.121917759
+0200
@@ -81,7 +81,9 @@
 	}
 
 	/* make sure two hierarchy separators aren't next to each others */
-	for (p = storage_name+1; *p != '\0'; p++) {
+	for (p = storage_name; *p != '\0'; p++) {
+		if (p == storage_name)
+			continue;
 		if (p[0] == ns->real_sep && p[-1] == ns->real_sep) {
 			client_send_tagline(cmd, "NO Invalid mailbox name.");
 			return NULL;


Cheers,
Christoph Bu?enius

-- 
Christoph Bu?enius
Rechnerbetriebsgruppe der Fakult?ten Informatik und Mathematik
TU M?nchen
+49 89-289-18519 <> Raum 00.05.055 <> Boltzmannstr. 3 <>
Garching

Hi,

On 26.9.2011, at 19.56, Christoph Bussenius wrote:
>    /* make sure two hierarchy separators aren't next to each others */
>    for (p = storage_name+1; *p != '\0'; p++) {
>    	if (p[0] == ns->real_sep && p[-1] == ns->real_sep) {
>    		client_send_tagline(cmd, "NO Invalid mailbox name.");
>    		return NULL;
>    	}
>    }
> 
> The loop iterates over the string storage_name starting from its second
> byte.  However, the string may be "".  This is the case if you
select
> the root of a namespace, like in "SELECT shared" or
> "SELECT shared/user1".  In that case, the code will read past the
end of
> the buffer from random memory until it finds a zero byte or a duplicate
> separator.
I finally managed to get this far in my mail backlog. :) Yes, that's a bug.
Fixed now slightly differently than you:
http://hg.dovecot.org/dovecot-2.0/rev/d406e376f8ee