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
Show replies by date
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