subversion-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bert Huijben" <b...@qqmail.nl>
Subject RE: Potential issue in libsvn_diff:diff_file.c:find_identical_prefix
Date Thu, 07 Jun 2012 09:47:09 GMT


> -----Original Message-----
> From: Bert Huijben [mailto:bert@qqmail.nl]
> Sent: donderdag 7 juni 2012 11:34
> To: 'Daniel Widenfalk'; dev@subversion.apache.org;
> users@subversion.apache.org
> Subject: RE: Potential issue in
libsvn_diff:diff_file.c:find_identical_prefix
> 
> 
> 
> > -----Original Message-----
> > From: Daniel Widenfalk [mailto:Daniel.Widenfalk@iar.se]
> > Sent: donderdag 7 juni 2012 11:06
> > To: dev@subversion.apache.org; users@subversion.apache.org
> > Subject: Potential issue in
libsvn_diff:diff_file.c:find_identical_prefix
> >
> > Hi,
> >
> > First off: I'm sorry if I post this in the wrong way.
> >
> > I've found a potential issue in the function "find_identical_prefix"
> > in libsvn_diff/diff_file.c
> >
> > The faulty code looks like this:
> >
> > diff_file.c:432 (as per 1.7.1, code identical to 1.7.5)
> > -------------------------------
> >       is_match = TRUE;
> >       for (delta = 0; delta < max_delta && is_match; delta +=
> > sizeof(apr_uintptr_t))
> >         {
> >           apr_uintptr_t chunk = *(const apr_size_t *)(file[0].curp +
> delta);
> >           if (contains_eol(chunk))
> >             break;
> >
> >           for (i = 1; i < file_len; i++)
> >             if (chunk != *(const apr_size_t *)(file[i].curp + delta))
> >               {
> >                 is_match = FALSE;
> >                 delta -= sizeof(apr_size_t);
> >                 break;
> >               }
> >         }
> > -------------------------------
> >
> > The problem is that the 64-bit build I'm using (ColabNet) have
> > different sizes for apr_uintptr_t and apr_size_t.
> >
> > From looking at the disassembly I can deduce that
> > sizeof(apr_uintptr_t) = 4 and sizeof(apr_size_t) = 8. This leads
> > to these two issues:
> >
> > 1) Data is truncated in the initial read-up to "chunk" and the compare
> >    in the inner loop will fail because the second read-up will compare
> >    a 64-bit value against a 32-bit value.
> >
> > 2) When the test fails it will back up delta by 8, not 4, resulting in
> >    a buffer advance of -4 later in the code. Once the search code has
> >    advanced another 4 character if will be back at the same spot.
> >
> >    Rinse and repeat.
> >
> > Are these a known issues?
> >
> > In my case this results in an infinite loop on the following input
> > string:
> >
> >   23 0a 23 20 54 68 69 73 20 70 72 6f 6a 65 63
> >
> > I found this out when my svn-client spiraled into an infinite loop
> > and would not respond to ctrl-c during a "svn up" command.
> 
> Which apr version did you use?
> 
> I think this issue was fixed in Subversion 1.7.2:
> 
> (From http://svn.apache.org/repos/asf/subversion/tags/1.7.2/CHANGES)
> * properly define WIN64 on Windows x64 builds (r1188609)
> 
> Not by a code change here in this piece of the code, but by making sure
that
> the pointer size is defined correctly in apr.
> So that would fix this issue and maybe several others.

And since then then also APR was patched to properly check for _WIN64
instead of WIN64 for defining these variable types correctly.

	Bert
> 
> 	Bert
> 



Mime
View raw message