You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Roland Ruedenauer <Ro...@yellow-computing.de> on 2004/10/25 16:59:37 UTC
Possibly wrong string comparisons in SVN
There seem to be some places in the SVN code where inadvertendly a
string prefix match is performed where a full match was desired.
1) libsvn_ra_local/split_url.c line 64
if ((strncmp (hostname, "localhost", 9) != 0))
2) libsvn_repos/fs-wrap.c line 330 and 332
/* Only svn:author and svn:date are fetchable. */
if ((strncmp (propname, SVN_PROP_REVISION_AUTHOR,
strlen(SVN_PROP_REVISION_AUTHOR)) != 0)
&& (strncmp (propname, SVN_PROP_REVISION_DATE,
strlen(SVN_PROP_REVISION_DATE)) != 0))
*value_p = NULL;
Other places?
--
Roland Rüdenauer
____________
Virus checked by inet.yellow-computing.de
Version: AVK 15.0.654, 25.10.2004
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Possibly wrong string comparisons in SVN
Posted by Andy Whitcroft <ap...@shadowen.org>.
Branko Čibej wrote:
> Roland Ruedenauer wrote:
>> if ((strncmp (hostname, "localhost", 9) != 0))
>>
>> 2) libsvn_repos/fs-wrap.c line 330 and 332
>>
>> /* Only svn:author and svn:date are fetchable. */
>> if ((strncmp (propname, SVN_PROP_REVISION_AUTHOR,
>> strlen(SVN_PROP_REVISION_AUTHOR)) != 0)
>> && (strncmp (propname, SVN_PROP_REVISION_DATE,
>> strlen(SVN_PROP_REVISION_DATE)) != 0))
>> *value_p = NULL;
>>
>>
> Probably, but they're all correct (I hope :-).
Although yes we are looking at a substring, should we not be checking
the terminator also? We'll get a false negative on the localhost check
localhost-foo.bar.com. I guess it should check for end of string or "."
as the next character? In the second case if you had other props with a
common prefix could they not suffer value leak (this is part of the
security fix right); obviously unlikely you'd choose to share these
prefixes, but perhaps its still 'wrong' per-see.
-apw
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Possibly wrong string comparisons in SVN
Posted by Branko Čibej <br...@xbc.nu>.
Roland Ruedenauer wrote:
>There seem to be some places in the SVN code where inadvertendly a
>string prefix match is performed where a full match was desired.
>
>1) libsvn_ra_local/split_url.c line 64
>
> if ((strncmp (hostname, "localhost", 9) != 0))
>
>2) libsvn_repos/fs-wrap.c line 330 and 332
>
> /* Only svn:author and svn:date are fetchable. */
> if ((strncmp (propname, SVN_PROP_REVISION_AUTHOR,
> strlen(SVN_PROP_REVISION_AUTHOR)) != 0)
> && (strncmp (propname, SVN_PROP_REVISION_DATE,
> strlen(SVN_PROP_REVISION_DATE)) != 0))
> *value_p = NULL;
>
>
Look again. We can't use strcmp in those places because we're looking at
a substring in a larger string. And you'll notice that the length of the
"prefix" for strncmp is always the same as the length of the string
we're comparig with.
>Other places?
>
>
Probably, but they're all correct (I hope :-).
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Possibly wrong string comparisons in SVN
Posted by Branko Čibej <br...@xbc.nu>.
Julian Foad wrote:
> Roland Ruedenauer wrote:
>
>> There seem to be some places in the SVN code where inadvertendly a
>> string prefix match is performed where a full match was desired.
>>
>> 1) libsvn_ra_local/split_url.c line 64
>>
>> if ((strncmp (hostname, "localhost", 9) != 0))
>
>
> Well caught. Thanks. I have confirmed this: "svn ls
> file://localhoster.collab.net/home/julianfoad/vcs/sandbox/" proceeds
> to list the contents of my local test repository.
Hmh! That should teach me to carefully look at the code (again -- I've
looked at that code quite a few times...) before answering.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Possibly wrong string comparisons in SVN
Posted by Julian Foad <ju...@btopenworld.com>.
Roland Ruedenauer wrote:
> There seem to be some places in the SVN code where inadvertendly a
> string prefix match is performed where a full match was desired.
>
> 1) libsvn_ra_local/split_url.c line 64
>
> if ((strncmp (hostname, "localhost", 9) != 0))
Well caught. Thanks. I have confirmed this: "svn ls
file://localhoster.collab.net/home/julianfoad/vcs/sandbox/" proceeds to list
the contents of my local test repository.
> 2) libsvn_repos/fs-wrap.c line 330 and 332
>
> /* Only svn:author and svn:date are fetchable. */
> if ((strncmp (propname, SVN_PROP_REVISION_AUTHOR,
> strlen(SVN_PROP_REVISION_AUTHOR)) != 0)
> && (strncmp (propname, SVN_PROP_REVISION_DATE,
> strlen(SVN_PROP_REVISION_DATE)) != 0))
Would you like to prepare a patch that fixes these, with a log message, in the
style documented in "HACKING"? That would be very helpful; but if you don't
have time, just say so.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org