Re: [PATCH] Fix unitialized memory access in svn_canonicalize_path()

Am Samstag, den 21.06.2008, 21:34 -0400 schrieb Karl Fogel:
> Jelmer Vernooij <jelmer_at_samba.org> writes:
> > svn_path_canonicalize() will try to access a single byte before its
> > allocated buffer if the path specified is "". The attached patch fixes
> > this. I've confirmed the error and the fix with valgrind.
> >
> > [[[
> > * subversion/libsvn_subr/path.c (svn_canonicalize_path): Avoid
> > accessing unitialized memory when path is "".
> > ]]]
>
> Your fix looks correct to me. But I think path=="" is the only case
> where your dst > canon check would get invoked anyway. If so, a better
> fix might be to just test for the special case at the top of the
> function:
>
> Index: subversion/libsvn_subr/path.c
> ===================================================================
> --- subversion/libsvn_subr/path.c (revision 31834)
> +++ subversion/libsvn_subr/path.c (working copy)
> @@ -1248,6 +1248,10 @@
> apr_size_t canon_segments = 0;
> svn_boolean_t uri;
>
> + /* "" is already canonical */
> + if (! *path)
> + return path;
> +
> dst = canon = apr_pcalloc(pool, strlen(path) + 1);
>
> /* Copy over the URI scheme if present. */
>
> Thoughts?
Yeah, I agree that's a bit clearer way of handling it.

Cheers,

Jelmer

-- 
Jelmer Vernooij <jelmer@samba.org> - http://samba.org/~jelmer/
Jabber: jelmer_at_jabber.fsfe.org