[libvirt PATCH] remote: Avoid crash in remoteSplitURIScheme()

Andrea Bolognani abologna at redhat.com
Fri Dec 10 13:47:41 UTC 2021


On Fri, Dec 10, 2021 at 11:31:19AM +0100, Peter Krempa wrote:
> On Fri, Dec 10, 2021 at 10:59:27 +0100, Andrea Bolognani wrote:
> > @@ -69,7 +69,15 @@ remoteSplitURIScheme(virURI *uri,
> >                       char **driver,
> >                       remoteDriverTransport *transport)
> >  {
> > -    char *p = strchr(uri->scheme, '+');
> > +    char *p = NULL;
> > +
> > +    if (!uri->scheme) {
> > +        virReportError(VIR_ERR_INVALID_ARG, "%s",
> > +                       _("missing scheme for URI"));
>
> The other place which leads to the call of this helper (virConnectOpenInternal)
> uses the following error to reject the uri if scheme is missing:
>
> virReportError(VIR_ERR_NO_CONNECT,
>                _("URI '%s' does not include a driver name"),
>                name);

Yeah, it seems safer to catch the issue inside the helper than
requiring the callers to perform the check ahead of time. It's okay
for virConnectOpen() to have a nicer error message, as it's the one
that people are more likely to see.

I entertained the thought of adding the check to virURIParse()
directly, because I can't think of a scenario where having a NULL
scheme would be considered valid. But that seemed like a change that
had the potential to break unrelated stuff, so I cowardly decided to
go with the safe version instead O:-)

> It looks like it's unlikely that anybody would use virt-ssh-helper
> manually though.

Absolutely! But it's still in the default $PATH, and not crashing
when passed invalid arguments is what I would consider the bare
minimum :)

Thanks for the review!

-- 
Andrea Bolognani / Red Hat / Virtualization




More information about the libvir-list mailing list