[Libguestfs] [nbdkit PATCH] iso: Shell-quote an alternative isoprog
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(a)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.
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