[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

RE: Potential issue in libsvn_diff:diff_file.c:find_identical_prefix

From: Bert Huijben <bert_at_qqmail.nl>
Date: Thu, 7 Jun 2012 11:47:09 +0200

> -----Original Message-----
> From: Bert Huijben [mailto:bert_at_qqmail.nl]
> Sent: donderdag 7 juni 2012 11:34
> To: 'Daniel Widenfalk'; dev_at_subversion.apache.org;
> users_at_subversion.apache.org
> Subject: RE: Potential issue in
libsvn_diff:diff_file.c:find_identical_prefix
>
>
>
> > -----Original Message-----
> > From: Daniel Widenfalk [mailto:Daniel.Widenfalk_at_iar.se]
> > Sent: donderdag 7 juni 2012 11:06
> > To: dev_at_subversion.apache.org; users_at_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
>
Received on 2012-06-07 11:47:55 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.