This is the mail archive of the
cygwin-apps@cygwin.com
mailing list for the Cygwin project.
Re: [PATCH] Re: New bug added to README
- From: Igor Pechtchanski <pechtcha at cs dot nyu dot edu>
- To: Robert Collins <rbcollins at cygwin dot com>
- Cc: cygwin-apps at cygwin dot com
- Date: Mon, 21 Apr 2003 14:15:23 -0400 (EDT)
- Subject: Re: [PATCH] Re: New bug added to README
- Reply-to: cygwin-apps at cygwin dot com
On 22 Apr 2003, Robert Collins wrote:
> Hmm, I never saw the prior email. Lost in the ether I guess.
>
> On Mon, 2003-04-21 at 23:15, Igor Pechtchanski wrote:
> > Resending with a better subject this time. Oh, and "ping".
> > Igor
> >
> > ---------- Forwarded message ----------
> > Date: Thu, 17 Apr 2003 10:08:16 -0400 (EDT)
> > From: Igor Pechtchanski <pechtcha at cs dot nyu dot edu>
> > Reply-To: cygwin-apps at cygwin dot com
> > To: Max Bowsher <maxb at ukf dot net>
> > Cc: cygwin-apps at cygwin dot com
> > Subject: Re: New bug added to README
> >
> > On Thu, 17 Apr 2003, Max Bowsher wrote:
> >
> > > maxb wrote:
> > > > CVSROOT: /cvs/cygwin-apps
> > > > Module name: setup
> > > > Changes by: maxb 2003-04-17 08:41:41
> > > >
> > > > Log message:
> > > > New bug in TODO:
> > > >
> > > > * Audit rfc1738 code for bad memory/string handling. Example: Crash occurs
> > > > if rfc1738 encoded dirname is truncated in the middle of a %xx sequence.
> > >
> > > Suggesting this be considered for Release Blocker status.
> > > Max.
> >
> > Yup, there's a bug all-right:
> >
> > rfc1738.cc, in rfc1738_unescape() [line 201]:
> > for (i = j = 0; s[j]; i++, j++)
> > {
> > s[i] = s[j];
> > if (s[i] != '%')
> > continue;
> > if (s[j + 1] == '%')
> > { /* %% case */
> > j++;
> > continue;
> > }
> > > if (s[j + 1] && s[j + 2])
> >
> > It will crash in the line above, since it overruns the buffer (by 2). I'm
> > attaching a patch. Perhaps the squid people should also be notified.
>
> Well, thats me.
>
> However, I don't see the bug. I've written some test cases, and I cannot
> get the routine to fail.
>
> logic:
>
> the string is NULL terminated.
> on loop entry, s[j] is not NULL, so s[j+1] must be valid.
> if (s[j+1] && s[j + 2]) cannot read outside the buffer, as the
> left->right boolearn evaluation checks that
> s[j+1] (known to be a valid address) is not NULL, which implies that
> s[j+2] must be valid (as it at worst will be the terminating NULL).
>
> As a shorthand for this, consider that all your patch does is move the
> test for s[j+1] or s[j+2] being NULL from guarding the unescape logic to
> an explicit skip.
Ouch, you're quite right. I'm sorry for the hasty conclusion - I only
glanced at the code and thought I saw a bug.
> Igor, have you done an assembly dump to see exactly what is occuring? My
> WAG is a comiler optimisation bug.
> Rob
No, I haven't. In fact, I'm unable to reproduce the crash, so I'll just
shut up now.
Igor
--
http://cs.nyu.edu/~pechtcha/
|\ _,,,---,,_ pechtcha at cs dot nyu dot edu
ZZZzz /,`.-'`' -. ;-;;,_ igor at watson dot ibm dot com
|,4- ) )-,_. ,\ ( `'-' Igor Pechtchanski
'---''(_/--' `-'\_) fL a.k.a JaguaR-R-R-r-r-r-.-.-. Meow!
Knowledge is an unending adventure at the edge of uncertainty.
-- Leto II