<<< Date Index >>>     <<< Thread Index >>>

Re: [Fwd: mutt buffer overflow]



* Thu Aug 18 2005 Thomas Roessler <roessler@xxxxxxxxxxxxxxxxxx>

> The assumption that is made in mutt_decode_xbit is that
> mutt_convert_to_state actually decreases the l counter by at least
> one -- if l is sufficiently large.
> 
> This maps to a similar assumption that mutt_iconv doesn't return
> without at least consuming one byte.
> 
> mutt_convert_to_state invokes mutt_iconv with inrepls set to 0, and
> outrepl being "?", which immediately leaves out a lot of the
> complexity in that function.
> 
> Reading through it, the one way in which that function does not
> consume any input is when the input buffer begins with an incomplete
> multibyte sequence.  Our assumption now becomes that any string of
> sufficient length either begins with a valid (hence consumed, at
> least partially) or invalid (hence skipped) multibyte sequence, but
> not with an incomplete one.

So what should we do?
Which should be fixed - mutt_iconv, mutt_convert_to_state, or
(mutt_decode_uuencoded and) mutt_decode_xbit?
We can't use inrepl/outrepl tricks in other functions than
mutt_iconv. But mutt_iconv would affect many other places of mutt.

If the input buffer is longer than MB_LEN_MAX, at least one byte
must be consumed anyway. And BUFI_SIZE == 1000 > MB_LEN_MAX.
So, mutt_convert_to_state in mutt_decode_xbit must consume at
least one byte.


Attachment: An untested patch for mutt_convert_to_state.
-- 
tamo
--- handler.c.BAK       Sat Aug 20 21:49:43 2005
+++ handler.c   Sat Aug 20 22:02:59 2005
@@ -80,6 +80,7 @@ void mutt_convert_to_state(iconv_t cd, c
   ICONV_CONST char *ib;
   char *ob;
   size_t ibl, obl;
+  int must_decrease = 0;
 
   if (!bufi)
   {
@@ -100,6 +101,9 @@ void mutt_convert_to_state(iconv_t cd, c
     return;
   }
 
+  if (*l >= MB_LEN_MAX)
+    must_decrease = 1;
+
   ib = bufi, ibl = *l;
   for (;;)
   {
@@ -108,6 +112,14 @@ void mutt_convert_to_state(iconv_t cd, c
     if (ob == bufo)
       break;
     state_prefix_put (bufo, ob - bufo, s);
+  }
+
+  if (must_decrease && ibl >= *l)
+  {
+    iconv (cd, 0, 0, 0, 0); /* ??? */
+    state_prefix_put ("?", 1, s);
+    ibl = *l - 1;
+    ib = bufi + 1;
   }
   memmove (bufi, ib, ibl);
   *l = ibl;