[Libguestfs] [nbdkit PATCH] iso: Shell-quote an alternative isoprog
Richard W.M. Jones
rjones at redhat.com
Wed Jun 26 17:16:31 UTC 2019
On Wed, Jun 26, 2019 at 11:53:04AM -0500, Eric Blake wrote:
> Otherwise, a user can do things like "nbdkit iso . prog='date;prog'"
> to run unintended commands in addition to their alternative isoprog.
> This is not a CVE (since nbdkit isn't running with any more privileges
> than the user running those commands themselves), but shows the
> frailty of relying on the shell to parse subsidiary commands rather
> than exec()ing them directly. This patch also doesn't resolve the
> fact that we are also passing params= through shell parsing (if we
> don't like that, we should consider changing the interface to make the
> user write param='-V' param='My Disk Image' and use shell_quote() over
> each param, rather than the current params='-V "My Disk Image"'), but
> does try to enhance the docs to point it out with more clarity.
>
> Signed-off-by: Eric Blake <eblake at redhat.com>
> ---
>
> I'm pushing this now, but we may want to reconsider the iso plugin
> exposing params= that is intentionally designed for another round of
> shell parsing, as a followup patch. Ideally, we want to avoid ever
> passing user-supplied data through another shell invocation without
> first re-quoting it.
This is fine. I think users can use params if they want to add
extra parameters.
ACK
Thanks, Rich.
> plugins/iso/nbdkit-iso-plugin.pod | 4 +++-
> plugins/iso/iso.c | 3 ++-
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/plugins/iso/nbdkit-iso-plugin.pod b/plugins/iso/nbdkit-iso-plugin.pod
> index 90e26f01..597c3c50 100644
> --- a/plugins/iso/nbdkit-iso-plugin.pod
> +++ b/plugins/iso/nbdkit-iso-plugin.pod
> @@ -62,7 +62,9 @@ For example:
> would specify Joliet (I<-J>), Rock Ridge (I<-r>) and TRANS.TBL (I<-T>)
> extensions, and specify the volume ID (I<-V>) as C<My Disk Image>.
>
> -Take care when quoting this parameter.
> +Take care when quoting this parameter; nbdkit passes the resulting
> +string through another layer of shell interpretation without any
> +sanity checks for unquoted shell metacharacters.
>
> =item B<prog=>mkisofs
>
> diff --git a/plugins/iso/iso.c b/plugins/iso/iso.c
> index 4728ff32..5634bac9 100644
> --- a/plugins/iso/iso.c
> +++ b/plugins/iso/iso.c
> @@ -94,7 +94,8 @@ make_iso (void)
> return -1;
> }
>
> - fprintf (fp, "%s -quiet", isoprog);
> + shell_quote (isoprog, fp);
> + fprintf (fp, " -quiet");
> if (params)
> fprintf (fp, " %s", params);
> for (i = 0; i < nr_dirs; ++i) {
> --
> 2.20.1
>
> _______________________________________________
> Libguestfs mailing list
> Libguestfs at redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs
--
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
More information about the Libguestfs
mailing list