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;