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

On Sun, 22 Jun 2008, Jelmer Vernooij wrote:

> 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.

This was committed to trunk in r31837.