[PATCH] Re: New bug added to README

Igor Pechtchanski pechtcha@cs.nyu.edu
Mon Apr 21 18:15:00 GMT 2003


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@cs.nyu.edu>
> > Reply-To: cygwin-apps@cygwin.com
> > To: Max Bowsher <maxb@ukf.net>
> > Cc: cygwin-apps@cygwin.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@cs.nyu.edu
ZZZzz /,`.-'`'    -.  ;-;;,_		igor@watson.ibm.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



More information about the Cygwin-apps mailing list