[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-pim
Subject:    Re: [Kde-pim] OOM in kio_imap4 get
From:       Will Stephenson <wstephenson () kde ! org>
Date:       2006-02-16 21:34:08
Message-ID: 200602162234.08740.wstephenson () kde ! org
[Download RAW message or body]

On Thursday 16 February 2006 21:21, Ingo Klöcker wrote:
> On Thursday 16 February 2006 18:32, Will Stephenson wrote:
> > On Thursday 16 February 2006 16:19, Will Stephenson wrote:
> > > However I'm not sure yet why QAsciiDict blows up when adding a
> > > parameter (mimeHeader::setParameter()) with key (aLabel)
> > >
> > > =?windows-1252?Q?NEED=5FGro=DFhandels-Newsletter=5F25=2E11=2E05=2Ep
> > >df?=
> > >
> > > and value (aValue)
> > >
> > > FILENAME
> > >
> > > so I'll keep looking to see what the truncation code does wrong and
> > > then commit.
> >
> > mimeHeader::setParameter() in kdepim/ioslaves/imap4/mimeheader.cpp
> > can endless loop and OOM if presented with a very long key not
> > containing * or % and a short value.
>
> A long key should never occur (see my other message; key and value seem
> to be swapped for some reason).

The patch attached to my mail at 1619CET fixes this; the code was a bit 
cavalier with its expectation of the execution order of two parse calls 
embedded in a call to setParameter().

> OTOH, nothing prevents an attacker from creating a message with a long
> key so this bug seems to indicate that kio_imap4 is vulnerable to a DOS
> attack. The only sensible fix for this is to truncate long keys.
> Neither RFC 204[5-7] nor RFC 2231 specify a maximal length for the name
> of a parameter, but since a line should not be longer than 80
> characters (including CRLF) I'd say a maximal length of 60 for the name
> of a parameter is a good enough value. So the first line in
> mimeHeader::setParameter() should be something like
> aLabel.truncate(60); // truncate overly long parameter-names
> (which doesn't work because aLabel is const, but you get the point).

As you'll be aware the code does truncate the parameters, but fails to 
terminate for some inputs.  I didn't really grok how setParameter() works but 
given your explanation of continuations below and with rfc2231 I can probably 
fix it now.

> > The patch I posted earlier
> > fixes the encapsulating bug that causes this in imapparser.cpp, but I
> > would like to fix the warhead as well as the fuze.
> >
> > However I don't know enough about what how it should behave to fix
> > it, specifically in the code to truncate a long parameter key/value
> > pair:
> >
> > What is the encoding for - is the goal of the encoding just to escape
> > * so that the * used in the generated shortLabel does not clash?
>
> No, the encoding is there to encode any characters which are not allowed
> in a value, i.e. some characters which have a special meaning and all
> 8-bit characters.
>
> > What's the relationship between * in the label and * in the value?
>
> A * in a label indicates a continued/encoded parameter. A * in the value
> just needs to be escaped/encoded. The relevant RFC is RFC 2231 (MIME
> Parameter Value and Encoded Word Extensions: Character Sets, Languages,
> and Continuations). I quote an example:
> =====
>    For example, the content-type field
>
>         Content-Type: message/external-body; access-type=URL;
>          URL*0="ftp://";
>          URL*1="cs.utk.edu/pub/moore/bulk-mailer/bulk-mailer.tar"
>
>    is semantically identical to
>
>         Content-Type: message/external-body; access-type=URL;
>           URL="ftp://cs.utk.edu/pub/moore/bulk-mailer/bulk-mailer.tar"
> =====
>
> > Is the sense of the find in " if (encoded && aLabel.find ('*') == -1)
> > " correct?  This would only encode if * is not found in the label.
>
> My guess is that if label contains a '*' then the value is assumed to be
> already encoded and thus doesn't need to be encoded anymore.
>
> > What if the value contains encoded chars but they are encoded with in
> > =3C style not the %3C generated by our encoder?  Should these be
> > respected as well?
>
> Hmm, looks like one of those Microsoftian "let's ignore the standard and
> create our own". The following is a mime header with correctly encoded
> parameters (from RFC 2231):
>         Content-Type: application/x-stuff;
>          title*=us-ascii'en-us'This%20is%20%2A%2A%2Afun%2A%2A%2A
>
> In KMail we have some code which handles Microsoft's legacy encoding.
> I'm not sure what we should do here.
>
> > Why is the parameter inserted inside the while loop?  Since there is
> > no continue it may be inserted more than once.
>
> If you are talking about the while-loop in setParameter() then the
> answer is that the parameter is split into a continued parameter like
> URL in the above example (shortLabel = aLabel + '*' + a number).
>
> Regards,
> Ingo
_______________________________________________
kde-pim mailing list
kde-pim@kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
kde-pim home page at http://pim.kde.org/

[prev in list] [next in list] [prev in thread] [next in thread]