gmane.mail.mutt.devel

Lars Hecking | 18 Aug 20:22

[Fwd: mutt buffer overflow]

----- Forwarded message from Peter Valchev <pvalchev <at> sightly.net> -----

Mailing-List: contact bugtraq-help <at> securityfocus.com; run by ezmlm
Precedence: bulk
Delivered-To: mailing list bugtraq <at> securityfocus.com
Delivered-To: moderator for bugtraq <at> securityfocus.com
Date: Thu, 18 Aug 2005 02:57:33 -0600
From: Peter Valchev <pvalchev <at> sightly.net>
To: bugtraq <at> securityfocus.com, full-disclosure <at> lists.grok.org.uk
Subject: mutt buffer overflow
Mail-Followup-To: bugtraq <at> securityfocus.com,
	full-disclosure <at> lists.grok.org.uk
User-Agent: Mutt/1.4.2i

Summary/Impact:
There is a buffer overflow in mutt found thanks to ProPolice, which may
allow an attacker to execute code by sending a maliciously crafted email.
All latest versions appear affected.  Mutt is an e-mail client
that sucks less according to the headline on http://www.mutt.org/

Details:
The problem is in the mutt attachment/encoding/decoding functions,
specifically handler.c:mutt_decode_xbit() and the buffer
bufi[BUFI_SIZE].  The variable 'l' is used as a counter to reference a
position in the buffer and under certain circumstances its value can be
manipulated and becomes much larger than the size of this buffer, thus
overwriting other memory with many possible consequences.  This counter
should never exceed the size and I believe the logic in the
convert_to_state() function is supposed to reset it to 0, however
there is a flaw - I have included a possible fix but I'm not sure

(Continue reading)

headers

Thomas Roessler | 18 Aug 22:32

Re: [Fwd: mutt buffer overflow]

On 2005-08-18 19:25:54 +0100, Lars Hecking forwarded from bugtraq:

> overwriting other memory with many possible consequences.  This
> counter should never exceed the size and I believe the logic in
> the convert_to_state() function is supposed to reset it to 0,
> however there is a flaw - I have included a possible fix but I'm
> not sure it's the 100% correct fix and there seem to be no
> developers willing to fix this so far.  There are other functions
> affected in the same way due to copy/paste, such as
> mutt_decode_uuencoded() that this patch should also fix.

To begin with, neither of the proof of concept examples [1], [2]
posted to bugtraq leads to a buffer overflow here (latest CVS built
on Fedora Core 4).

1. ftp://ftp.00f.net/misc/mutt-crash-poc.mbox
2. http://sightly.net/peter/tmp/mutt-bug

So Frank's observation that this might be tied to a particular iconv
implementation seems to be valid.  Still, the question is if mutt is
making assumptions that it shouldn't make, or if that iconv
implementation is broken.

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.

(Continue reading)

headers

Vincent Lefevre | 18 Aug 23:32

Re: [Fwd: mutt buffer overflow]

On 2005-08-18 22:32:27 +0200, Thomas Roessler wrote:
> So Frank's observation that this might be tied to a particular iconv
> implementation seems to be valid.  Still, the question is if mutt is
> making assumptions that it shouldn't make, or if that iconv
> implementation is broken.

Do you mean that Mutt is making assumptions that are not clearly
documented? I'd say that in doubt, Mutt shouldn't make any assumption,
in particular when there is a risk of buffer overflow.

--

-- 
Vincent Lefèvre <vincent <at> vinc17.org> - Web: <http://www.vinc17.org/>
100% accessible validated (X)HTML - Blog: <http://www.vinc17.org/blog/>
Work: CR INRIA - computer arithmetic / SPACES project at LORIA

headers

TAKAHASHI Tamotsu | 20 Aug 15:19

Re: [Fwd: mutt buffer overflow]

* Thu Aug 18 2005 Thomas Roessler <roessler <at> does-not-exist.org>

> 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.

(Continue reading)