Libguestfs - Jun 2017 - [PATCH] lib: create: Allow any [[:alnum:]]+ string as a backingfmt parameter.

If you use the libguestfs tools which open disk images read-only
(eg. virt-df), with formats such as 'vdi', then you will see an error:

  error: invalid value for backingformat parameter 'vdi'

This is because opening a disk image read-only will try to create a
qcow2 file with the original image as a backing file.  However the
list of permitted backing formats was very restrictive and did not
include 'vdi' (nor many other uncommon formats).

Instead of using a whitelist for backing formats, just validate that
the string is alphanumeric and short.

Thanks: Mike Goodwin for reporting the bug.
---
 lib/create.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/lib/create.c b/lib/create.c
index bd4c32ef7..fff5cf332 100644
--- a/lib/create.c
+++ b/lib/create.c
@@ -241,6 +241,14 @@ is_power_of_2 (unsigned v)
   return v && ((v & (v - 1)) == 0);
 }
 
+/**
+ * Check for valid backing format.  Allow any C<^[[:alnum]]+$>
+ * (in C locale), but limit the length to something reasonable.
+ */
+#define VALID_FORMAT(format) \
+  guestfs_int_string_is_valid ((format), 1, 16, \
+                               VALID_FLAG_ALPHA|VALID_FLAG_DIGIT, "")
+
 static int
 disk_create_qcow2 (guestfs_h *g, const char *orig_filename, int64_t size,
                    const char *backingfile,
@@ -267,12 +275,7 @@ disk_create_qcow2 (guestfs_h *g, const char *orig_filename,
int64_t size,
 
   if (optargs->bitmask & GUESTFS_DISK_CREATE_BACKINGFORMAT_BITMASK) {
     backingformat = optargs->backingformat;
-    /* Conservative whitelist.  This can be extended with other
-     * valid formats as required.
-     */
-    if (STRNEQ (backingformat, "raw") &&
-        STRNEQ (backingformat, "qcow2") &&
-        STRNEQ (backingformat, "vmdk")) {
+    if (!VALID_FORMAT (backingformat)) {
       error (g, _("invalid value for backingformat parameter ‘%s’"),
              backingformat);
       return -1;
-- 
2.13.0

On Thursday, 8 June 2017 09:19:09 CEST Richard W.M. Jones
wrote:
> If you use the libguestfs tools which open disk images read-only
> (eg. virt-df), with formats such as 'vdi', then you will see an
error:
> 
>   error: invalid value for backingformat parameter 'vdi'
> 
> This is because opening a disk image read-only will try to create a
> qcow2 file with the original image as a backing file.  However the
> list of permitted backing formats was very restrictive and did not
> include 'vdi' (nor many other uncommon formats).
> 
> Instead of using a whitelist for backing formats, just validate that
> the string is alphanumeric and short.
> 
> Thanks: Mike Goodwin for reporting the bug.
> ---
LGTM.

Should there be a new test for trying to open vdi images?

-- 
Pino Toscano