Philip Martin wrote on Tue, Nov 15, 2011 at 13:58:20 +0000:
> "Daniel Shahaf" <d.s_at_daniel.shahaf.name> writes:
>
> > On Tuesday, November 15, 2011 2:33 AM, philip_at_tigris.org wrote:
> >> http://subversion.tigris.org/issues/show_bug.cgi?id=4060
> >>
> >> ------- Additional comments from philip_at_tigris.org Tue Nov 15 02:33:17 -0800 2011 -------
> >> The doc string for svn_repos_authz_check_access doesn't tell us about
> >> repos_name=NULL but clearly 1.6 handled it. Given that mod_dav_svn passes "" I
> >> suppose svn_repos_authz_check_access could convert NULL to "", either here:
> >>
> > ...
> >>
> >> Both changes fix the bug.
> >>
> >> Or perhaps we should be changing is_applicable_section?
> >
> > I thought we have to change is_applicable_section() anyway for memory
> > correctness issues: the return() statement may read one byte before PATH_SPEC
> > (and potentially also a byte after SECTION_NAME's end).
>
> From IRC: No. The NULL repos_name in authz_get_tree_access
>
> baton.qualified_repos_path = apr_pstrcat(pool, repos_name,
> ":", path, (char *)NULL);
>
> cases qualified_repos_path to be truncated to "" which doesn't follow
> the rules for input to is_applicable_section. Converting NULL to ""
> means that the truncation doesn't occur and is_applicable_section gets
> input it can handle.
From IRC: And nonetheless, if someone ever writes code that does pass ""
to is_applicable_section() then it'll access random memory locations, so
I'd like to fix it anyway.
(added that to my list)
Received on 2011-11-15 15:21:29 CET