This is the mail archive of the cygwin-apps@cygwin.com mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Re: New bug added to README


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.

Igor, have you done an assembly dump to see exactly what is occuring? My
WAG is a comiler optimisation bug.

Rob

-- 
GPG key available at: <http://users.bigpond.net.au/robertc/keys.txt>.

Attachment: signature.asc
Description: This is a digitally signed message part


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]