On Thu, Oct 23, 2025 at 11:27:23AM +0100, Richard W.M. Jones via Libguestfs wrote: > On Tue, Oct 21, 2025 at 03:23:59PM -0500, Eric Blake via Libguestfs wrote: > > Dan Berrangé ran a free trial of zeropath (http://zeropath.com/) AI > > analysis on libnbd, and it highlighted the following: > > > > "When using nbd+ssh:// URIs the library constructs an argv array for > > ssh from parsed URI parts (server, port, user, unix socket, nbd-port) > > and execs it. The server component is used directly as an ssh > > argument; if it begins with '-' an attacker can inject ssh options > > (e.g. -oProxyCommand=...) that cause ssh to run local commands. There > > is no protection (such as rejecting leading '-' in server or inserting > > a '--' to stop option parsing), so an attacker who can supply the URI > > can cause local command execution in the client process." > > > > eg with this.... "nbdinfo nbd+ssh://-oProxyCommand=rm%20run.in" > > you'll get a failure to start the NBD connection, but it none the less > > deletes the file 'run.in' in the local working directory > > > > The RFCs are vague enough that it is not immediately obvious whether > > there is any possibility of a valid hostname with a leading - (see > > https://www.netmeister.org/blog/hostnames.html). Still, it is better > > to pass the user's string on to ssh's determination of a valid > > hostname (which does appear to reject leading -) rather than trying to > > teach libnbd what patterns to allow, and thereby avoid risking any > > pattern written in libnbd accidentally being too restrictive. Do this > > by using "--" to end ssh options before the hostname, but that in turn > > must come after any use of -oUser=. With this in place, we now get a > > sane error rather than spawning a calculator with: > > > > $ nbdinfo nbd+ssh://-oProxyCommand=gnome-calculator > > hostname contains invalid characters > > /home/eblake/libnbd/info/.libs/nbdinfo: nbd_connect_uri: recv: server > > disconnected unexpectedly > > > > See also Libvirt commit e4cb8500 (Aug 2017), which in turn was > > inspired by GIT security flaws > > (http://blog.recurity-labs.com/2017-08-10/scm-vulns). We have put out > > a request to Red Hat security on whether this warrants a CVE in > > libnbd; however, as the problem was easy to identify using only free > > AI resources, and the problem itself is relatively low priority (to > > exploit it, an attacker has to convince an admin to run a program that > > will use libnbd on an untrusted URI), so we are publishing this now > > rather than waiting for any embargo. If a CVE is assigned, it will be > > announced to the mailing list in a followup post. > > > > Signed-off-by: Eric Blake <[email protected]> > > CC: Daniel P. Berrangé <[email protected]> > > --- > > lib/uri.c | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/lib/uri.c b/lib/uri.c > > index 3b099fe5..7413c434 100644 > > --- a/lib/uri.c > > +++ b/lib/uri.c > > @@ -291,8 +291,6 @@ uri_connect_ssh (struct nbd_handle *h, const xmlURIPtr > > uri, > > const_string_vector_append (&ssh_command, "-p"); > > const_string_vector_append (&ssh_command, port_str); > > > > - const_string_vector_append (&ssh_command, uri->server); > > - > > /* SSH user always comes from the authority part of the URI. */ > > if (uri->user) { > > /* Don't allow any special tokens in the username which ssh might > > @@ -310,6 +308,10 @@ uri_connect_ssh (struct nbd_handle *h, const xmlURIPtr > > uri, > > const_string_vector_append (&ssh_command, ssh_user); > > } > > > > + const_string_vector_append (&ssh_command, "--"); > > + const_string_vector_append (&ssh_command, uri->server); > > + > > + /* All further argc beginning with - are options to nc. */ > > const_string_vector_append (&ssh_command, "nc"); > > if (unixsocket) { > > const_string_vector_append (&ssh_command, "-U"); > > Reviewed-by: Richard W.M. Jones <[email protected]> > > We'll need to do a bit of coordination here: > > - We will need to backport the change to libnbd 1.22. Because the > code changed substantially, the backport is effectively a new > patch.
I see that Eric has done this one already, thanks! Rich. > - Fedora will need to be updated <= I will do this when it goes upstream > > - Other Linux distro maintainers need to be notified <= I will do this now > > - RHEL will have to be updated, but I believe we're waiting on the > decision of whether this is a CVE before we can do that. > > Rich. > > -- > Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones > Read my programming and virtualization blog: http://rwmj.wordpress.com > libguestfs lets you edit virtual machines. Supports shell scripting, > bindings from many languages. http://libguestfs.org > _______________________________________________ > Libguestfs mailing list -- [email protected] > To unsubscribe send an email to [email protected] -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com nbdkit - Flexible, fast NBD server with plugins https://gitlab.com/nbdkit/nbdkit _______________________________________________ Libguestfs mailing list -- [email protected] To unsubscribe send an email to [email protected]
