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&gt; --- 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