Re: ANNOUNCE: balsa-2.3.4 released

[Pawel Salek <pawsa theochem kth se>]

On 07/08/05 19:24:39, Andrew Lau wrote:
> On Mon, Jul 04, 2005 at 11:30:37PM +0000, Pawel Salek wrote:
> > Major highlights of this balsa release wrt 2.3.3 are:
> > - possible buffer overflow in IMAP code fixed.
>
> Hi everyone,
>
> Balsa 2.3.4-1 Debian packages for i386 are currently awaiting
> uploading
> at http://www.cse.unsw.edu.au/~alau/debian/ as usual.
>
> Could someone please elaborate on the "possible buffer overflow" and
> specifically point out the affected code if it's present in 2.3.0. If
> it's serious enough, I'd like to call in the Debian Security audit
> team
> to take a look and see if a backported patch will be required for
> Sarge.

Evil imap server could in principle trick balsa into constructing a  
request that will overflow the stack with a string overflowing the  
stack with ten (iirc) characters from [0-9,:] range. The code contains  
a short buffer (ten characters long) to contain a text representation  
of single integer number but sometimes, two numbers representing the  
message numbers can be placed in the buffer. I cannot see a way of  
abusing this overflow for anything more than crashing balsa at best but  
I am not an security professional.
Pawel

Index: libbalsa/imap/imap-commands.c
===================================================================
RCS file: /cvs/gnome/balsa/libbalsa/imap/imap-commands.c,v
retrieving revision 1.68
retrieving revision 1.69
diff -u -r1.68 -r1.69
--- libbalsa/imap/imap-commands.c	10 Jun 2005 20:47:58 -0000	1.68
+++ libbalsa/imap/imap-commands.c	1 Jul 2005 01:41:10 -0000	1.69
@@ -35,7 +35,6 @@
 coalesce_seq_range(int lo, int hi, CoalesceFunc incl, void *data)
 {
   GString * res = g_string_sized_new(16);
-  char buf[10], *str;
   enum { BEGIN, LASTOUT, LASTIN, RANGE } mode = BEGIN;
   int seq;
   unsigned prev =0, num = 0;
@@ -44,11 +43,11 @@
     if(seq<=hi && (num=incl(seq, data)) != 0) {
       switch(mode) {
       case BEGIN: 
-        sprintf(buf, "%u", num); g_string_append(res, buf); 
+        g_string_append_printf(res, "%u", num);
         mode = LASTIN; break;
       case RANGE:
         if(num!=prev+1) {
-          sprintf(buf, ":%u,%u", prev, num); g_string_append(res, buf); 
+          g_string_append_printf(res, ":%u,%u", prev, num);
           mode = LASTIN;
         }
         break;
@@ -58,7 +57,7 @@
           break;
         } /* else fall through */
       case LASTOUT: 
-        sprintf(buf, ",%u", num); g_string_append(res, buf); 
+        g_string_append_printf(res, ",%u", num);
         mode = LASTIN; break;
       }
     } else {
@@ -67,21 +66,14 @@
       case LASTOUT: break;
       case LASTIN: mode = LASTOUT; break;
       case RANGE: 
-        sprintf(buf, ":%u", prev); g_string_append(res, buf); 
+        g_string_append_printf(res, ":%u", prev);
         mode = LASTOUT;
         break;
       }
     }
     prev = num;
   }
-  if(mode == BEGIN) {
-    str = NULL;
-    g_string_free(res, TRUE);
-  } else {
-    str = res->str;
-    g_string_free(res, FALSE);
-  }
-  return str;
+  return g_string_free(res, mode == BEGIN);
 }
 
 static unsigned