We need to make sure the URI scheme is present before passing it to strchr(), otherwise we're going to get $ virt-ssh-helper foo Segmentation fault (core dumped) Signed-off-by: Andrea Bolognani <abologna(a)redhat.com&gt; --- src/remote/remote_sockets.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/remote/remote_sockets.c b/src/remote/remote_sockets.c index 2979576680..c315b24d30 100644 --- a/src/remote/remote_sockets.c +++ b/src/remote/remote_sockets.c @@ -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"));

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:-)