[prev in list] [next in list] [prev in thread] [next in thread] 

List:       subversion-dev
Subject:    Re: Revision dates in URLs
From:       "C. Michael Pilato" <cmpilato () collab ! net>
Date:       2006-08-29 15:24:48
Message-ID: 44F45C40.3060808 () collab ! net
[Download RAW message or body]


Jonathan Gilbert wrote:
> At 02:22 PM 28/08/2006 -0400, C. Michael Pilato wrote:
>> Nilton Volpato wrote:
>>> Index: subversion/libsvn_subr/opt.c
>>> ===================================================================
>>> --- subversion/libsvn_subr/opt.c	(revision 21264)
>>> +++ subversion/libsvn_subr/opt.c	(working copy)
>>> @@ -730,9 +730,24 @@
>>>              }
>>>            else  /* looking at non-empty peg revision */
>>>              {
>>> +	      char *rev = path + i + 1;
>>> +	      int rev_len = strlen(rev);
>>> +	      char *unescaped_rev;
>>> +
>>> +	      /* if rev.startswith('%7B') and rev.endswith('%7D') */ 
>>> +	      if ( rev[0] == '%' && rev[1] == '7' && rev[2] == 'B' &&
>>> +		   rev[rev_len-3] == '%' && rev[rev_len-2] == '7' && 
>>> +		   rev[rev_len-1] == 'D' ) {
>> Eek!  There's no bounds-checking, so the patch can read off either end
>> of the string.  Remember, not all input to this function is URI-escaped
>> URLs -- could be regular ol' system paths, which quite possibly can be
>> named just "%".
> 
> Actually, if you look closely, this code is very clever:
> 
> - If the string is 0 bytes long, then "rev[0] == '%'" will fail, and none
> of the other checks will run.
> - If the string is 1 byte long, then "rev[1] == '7'" will fail, and none of
> the other checks will run.
> - If the string is 2 bytes long, then "rev[2] == 'B'" will fail, and none
> of the other checks will run.
> - Therefore, if you actually make it to the "rev[rev_len - 3] == '%'"
> check, you know that rev_len is at least 3.
> 
> There would be a small issue if it were, say, a quote character being
> checked, such that the start & end character would be the same and a string
> with only one character would pass both the starts-with and ends-with
> tests, but since the left brace ends in 'B' and the right brace ends in
> 'D', a string fewer than 6 bytes will never pass this test.

You are, of course, completely correct.  And I did, of course,
completely brain-O on this.

-- 
C. Michael Pilato <cmpilato@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


["signature.asc" (application/pgp-signature)]

[prev in list] [next in list] [prev in thread] [next in thread]