Re: [PATCH] qcow2: Forbid use of protocol: prefix on data_file

[Kevin Wolf <kwolf@redhat.com>]

Am 23.05.2025 um 20:20 hat Eric Blake geschrieben:
> Ever since CVE-2024-4467 (see commit 7ead9469 in qemu v9.1.0), we have
> intentionally treated command-line arguments as local files, and not
> protocol specifications (you have to specify backing files with
> full-blown QMP if it is intentional to access something more
> complicated).  However, that patch forgot about qcow2 data-file, which
> is another place where we really should not be hard-coding protocol
> names in the qcow2 metadata.
> 
> Fix this by changing the decision point on whether to allow protocols
> to each driver, rather than hard-coded to true in the generic code;
> qcow2 data_file is the only place where we change the former default
> of true.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>

This commit message is very confusing. Commit 7ead9469 was primarily
about qcow2 data files, it certainly didn't forget them. It also didn't
do something in other places, but not in qcow2. Another thing it wasn't
about is command line arguments, but it restricted the references stored
in (potentially untrusted) image files.

The main difference between it and this patch is that commit 7ead9469
was about opening images (which is a security problem because you might
deal with untrusted images), and this one is about creating images
(which has no such problem, you're creating the image only now).

Of course, if you can't open an image with protocol: syntax, it makes
sense that creating it with the same syntax fails, too, for consistency.
So I'm not opposed to this patch, but I think it needs a completely
different commit message.

> diff --git a/block/vmdk.c b/block/vmdk.c
> index 9c7ab037e14..576af241e59 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -2332,7 +2332,7 @@ vmdk_create_extent(const char *filename, int64_t filesize, bool flat,
>      int ret;
>      BlockBackend *blk = NULL;
> 
> -    ret = bdrv_co_create_file(filename, opts, errp);
> +    ret = bdrv_co_create_file(filename, opts, true, errp);
>      if (ret < 0) {
>          goto exit;
>      }

If we want to be consistent with opening, VMDK extents should pass
allow_protocol_prefix=false.

Kevin