[libvirt PATCH 11/11] virnetclient: Escape socket path

[Daniel P. Berrangé]

On Fri, Feb 11, 2022 at 06:39:19PM +0100, Andrea Bolognani wrote:
> Just like the name of the netcat command and the connection URI,
> the socket path is a user-provided piece of information that
> might contain characters that have special meaning for the
> shell, and as such should be escaped.
> 
> Signed-off-by: Andrea Bolognani <abologna at redhat.com>
> ---
>  src/rpc/virnetclient.c   |  5 +++--
>  tests/virnetsockettest.c | 18 +++++++++---------
>  2 files changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
> index 7e7e9d52a6..563cffff85 100644
> --- a/src/rpc/virnetclient.c
> +++ b/src/rpc/virnetclient.c
> @@ -422,6 +422,7 @@ virNetClientSSHHelperCommand(virNetClientProxy proxy,
>  {
>      g_autofree char *netcatPathSafe = virNetClientDoubleEscapeShell(netcatPath ? netcatPath : "nc");
>      g_autofree char *driverURISafe = virNetClientDoubleEscapeShell(driverURI);
> +    g_autofree char *socketPathSafe = virNetClientDoubleEscapeShell(socketPath);
>      g_autofree char *nccmd = NULL;
>      g_autofree char *helpercmd = NULL;
>  
> @@ -440,8 +441,8 @@ virNetClientSSHHelperCommand(virNetClientProxy proxy,
>          "else "
>              "ARG=; "
>          "fi; "
> -        "'%s' $ARG -U %s",
> -        netcatPathSafe, netcatPathSafe, socketPath);
> +        "'%s' $ARG -U '%s'",
> +        netcatPathSafe, netcatPathSafe, socketPathSafe);
>  
>      helpercmd = g_strdup_printf("virt-ssh-helper%s'%s'",
>                                  readonly ? " -r " : " ",

Again to avoid breaking apps we really want to minimize the change
here. IIUC the escaping should be a no-op for any scenario that
previously worked. So we if we avoid using ''  unless the path
contains whitespace we should be OK IIUC

> diff --git a/tests/virnetsockettest.c b/tests/virnetsockettest.c
> index b0e26bc2b2..d79027d637 100644
> --- a/tests/virnetsockettest.c
> +++ b/tests/virnetsockettest.c
> @@ -576,7 +576,7 @@ mymain(void)
>                           "else "
>                               "ARG=; "
>                           "fi; "
> -                         "'nc' $ARG -U /tmp/socket"
> +                         "'nc' $ARG -U '/tmp/socket'"
>                       "'\n",
>      };
>      if (virTestRun("SSH test 1", testSocketSSH, &sshData1) < 0)
> @@ -596,7 +596,7 @@ mymain(void)
>                           "else "
>                               "ARG=; "
>                           "fi; "
> -                         "'netcat' $ARG -U /tmp/socket"
> +                         "'netcat' $ARG -U '/tmp/socket'"
>                       "'\n",
>      };
>      if (virTestRun("SSH test 2", testSocketSSH, &sshData2) < 0)
> @@ -616,7 +616,7 @@ mymain(void)
>                           "else "
>                               "ARG=; "
>                           "fi; "
> -                         "'netcat' $ARG -U /tmp/socket"
> +                         "'netcat' $ARG -U '/tmp/socket'"
>                       "'\n",
>      };
>      if (virTestRun("SSH test 3", testSocketSSH, &sshData3) < 0)
> @@ -640,7 +640,7 @@ mymain(void)
>                           "else "
>                               "ARG=; "
>                           "fi; "
> -                         "'nc' $ARG -U /tmp/socket"
> +                         "'nc' $ARG -U '/tmp/socket'"
>                       "'\n",
>          .dieEarly = true,
>      };
> @@ -659,7 +659,7 @@ mymain(void)
>                           "else "
>                               "ARG=; "
>                           "fi; "
> -                         "'nc' $ARG -U /tmp/socket"
> +                         "'nc' $ARG -U '/tmp/socket'"
>                       "'\n",
>      };
>      if (virTestRun("SSH test 6", testSocketSSH, &sshData6) < 0)
> @@ -675,7 +675,7 @@ mymain(void)
>                           "else "
>                               "ARG=; "
>                           "fi; "
> -                         "'''\\''n c'\\'''' $ARG -U /tmp/sock et"
> +                         "'''\\''n c'\\'''' $ARG -U '''\\''/tmp/sock et'\\''''"
>                       "'\n",
>      };
>      if (virTestRun("SSH test 7", testSocketSSH, &sshData7) < 0)
> @@ -691,7 +691,7 @@ mymain(void)
>                           "else "
>                               "ARG=; "
>                           "fi; "
> -                         "'''\\''n'\\''\\'\\'''\\''c'\\'''' $ARG -U /tmp/sock'et"
> +                         "'''\\''n'\\''\\'\\'''\\''c'\\'''' $ARG -U '''\\''/tmp/sock'\\''\\'\\'''\\''et'\\''''"
>                       "'\n",
>      };
>      if (virTestRun("SSH test 8", testSocketSSH, &sshData8) < 0)
> @@ -707,7 +707,7 @@ mymain(void)
>                           "else "
>                               "ARG=; "
>                           "fi; "
> -                         "'''\\''n\"c'\\'''' $ARG -U /tmp/sock\"et"
> +                         "'''\\''n\"c'\\'''' $ARG -U '''\\''/tmp/sock\"et'\\''''"
>                       "'\n",
>      };
>      if (virTestRun("SSH test 9", testSocketSSH, &sshData9) < 0)
> @@ -725,7 +725,7 @@ mymain(void)
>                               "else "
>                                   "ARG=; "
>                               "fi; "
> -                             "'nc' $ARG -U /tmp/socket; "
> +                             "'nc' $ARG -U '/tmp/socket'; "
>                           "fi"
>                       "'\n"
>      };
> -- 
> 2.34.1
> 

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|