Philip Martin wrote on Tue, Nov 15, 2011 at 13:58:20 +0000: > "Daniel Shahaf" <d...@daniel.shahaf.name> writes: > > > On Tuesday, November 15, 2011 2:33 AM, phi...@tigris.org wrote: > >> http://subversion.tigris.org/issues/show_bug.cgi?id=4060 > >> > >> ------- Additional comments from phi...@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)
