David James wrote:
> On 11/3/05, Michael Sinz <Michael.Sinz@sinz.org> wrote:
>
>>On 11/3/05, Julian Foad <julianfoad@btopenworld.com> wrote:
>>
>>>Michael Sinz wrote:
>>>
>>>>Patch is already attached to 2423 and I believe it to be correct.
>>>
>>>Please could you remind us what this issue is about by mentioning its subject
>>>in the subject line? (Yes I know I could go to the issue and find out, but I'm
>>>probably not interested in it so I'm not going to.)
>>
>>Sorry - I too want that in the subject.
>>
>>I have updated the subject.
>>
>>Also, more directly, the issue has to do with the fact that the escaping
>>function called to make a valid URL does not deal with a ":" character in
>>a relative path URL, which is what mod_dav_svn generates in the HTML
>>and XML output. A simple change to call the same function with an
>>argument telling it that the URL is relative and will not have a full path
>>pre-pended to it fixes the problem. (Note that the call looks different
>>since a #define is used to make the secondary parameter optional)
>
>
> Thanks for your careful analysis, Michael! I agree, it is definitely a
> good idea to use ap_os_escape_path instead of ap_escape_uri for
> relative filename paths in mod_dav_svn. I would definitely like to see
> your patch committed to both trunk and the 1.3.x line.
Thanks. BTW - the same patch is also relevant in the 1.2.x line and may
be of interest for those who are worried about the potential hole.
> Looking at the code for mod_dav_svn, I see there are three uses of
> "apr_escape_uri":
>
> 1) ap_escape_uri(r->pool, r->uri) (in get_parentpath_resource)
> 2) ap_escape_uri(r->pool, r->uri) (in dav_svn_get_resource)
> 3) ap_escape_uri(entry_pool, href) (in dav_svn_deliver)
>
> In situations (1) and (2), we call "ap_escape_uri" on full URLs, so
> this is fine. (Right?) In situation (3), we call "apr_escape_uri" on a
> relative path, and thus we are vulnerable to malicious
> cross-site-links. Your patch fixes situation (3) to use
> ap_os_escape_path, which properly escapes colons in filenames. Looks
> good to me.
That was what I found too. And, just to make sure the list sees this,
the ap_escape_uri() is just aa #define to ap_os_escape_path() so it
is still the same routine, just with the correct flag to support
relative paths. (BTW - I think Apache mis-named the parameter, I had
to read the code twice to be sure of what they were intending it to do)
--
Michael Sinz Technology and Engineering Director/Consultant
"Starting Startups" mailto:michael.sinz@sinz.org
My place on the web http://www.sinz.org/Michael.Sinz
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Nov 4 02:03:17 2005