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

List:       kmail-devel
Subject:    Re: [Bug 121650] Crash when opening an spam mail from imap
From:       Ingo =?iso-8859-15?q?Kl=F6cker?= <kloecker () kde ! org>
Date:       2006-08-30 6:09:49
Message-ID: 200608300809.55417 () helena ! mathA ! rwth-aachen ! de
[Download RAW message or body]

[Attachment #2 (multipart/signed)]


Am Dienstag, 29. August 2006 21:16 schrieb Allen Winter:
> On Tuesday 29 August 2006 15:06, Allen Winter wrote:
> > On Tuesday 29 August 2006 14:22, Ingo Klöcker wrote:
> > > On Tuesday 29 August 2006 15:43, Allen Winter wrote:
> > > > On Tuesday 29 August 2006 09:23, Ingo Klöcker wrote:
> > > > > Am Dienstag, 29. August 2006 14:41 schrieb Allen Winter:
> > > > > > --- branches/KDE/3.5/kdepim/kmail/kmmessage.cpp
> > > > > > #578484:578485 @ -3985,6 +3985,9  @
> > > > > >
> > > > > > //---------------------------------------------------------
> > > > > >------ ---- ---------- void KMMessage::updateBodyPart(const
> > > > > > QString partSpecifier, const QByteArray & data) {
> > > > > > +  if ( !data.data() || !data.size() )
> > > > > > +    return;
> > > > > > +
> > > > > >    DwString content( data.data(), data.size() );
> > > > > >    if ( numBodyParts() > 0 &&
> > > > > >         partSpecifier != "0" &&
> > > > >
> > > > > This fix doesn't look correct. According to the backtrace the
> > > > > crash happens in the line
> > > > >   content.resize( content.length()-2 );
> > > > > because of an integer underflow. DwString::resize() is called
> > > > > with length aLen=4294967295 (see stack frame #12).
> > > > >
> > > > > A better fix would be something like
> > > > >   content.resize( QMAX( content.length(), 2 ) - 2 );
> > > >
> > > > But content seems hosed because the constructor is getting
> > > > bogus data().
> > >
> > > Don't know. It seems that data is of size 1 (because 1-2 == -1 ==
> > > 4294967295 in uint32). The comment before the resize, i.e. "get
> > > rid of EOL", seems to suggest that data should at least contain
> > > CRLF. Maybe for some reason the IMAP server doesn't add a
> > > trailing CRLF? Or it only adds an LF (which would be a violation
> > > of the IMAP spec which demands CRLF-line-endings)? I'm not sure
> > > whether it's worth to look further into this since the crash
> > > seems to be fixed now.
> >
> > Ahh.. so that makes sense.
> > I'm concerned then that we might be stripping 1 extra char if an LF
> > only is sent.
> >
> > Basically we want to strip off trailing whitespace.. .do you agree?
> > Too bad we have a DwString and not a QString.
>
> There is a DwString::Trim() method.  It seems to remove leading and
> trailing whitespace. It uses isspace() to determine whitespace.  Is
> it safe to remove leading whitespace?

I don't know how the text that can occur might look like. And even if we 
knew it we shouldn't expect all IMAP servers to do it right. So testing 
for a trailing CRLF or LF is probably the safest way to deal with this.

Regards,
Ingo

[Attachment #5 (application/pgp-signature)]

_______________________________________________
KMail developers mailing list
KMail-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmail-devel


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