Max Reitz <mre...@redhat.com> writes:
> On 2015-03-20 at 09:05, Markus Armbruster wrote:
>> Probing is convenient, but probing untrusted raw images is insecure
>> (CVE-2008-2004). To avoid it, users should always specify raw format
>> explicitly. This isn't trivial, and even sophisticated users have
>> gotten it wrong (libvirt CVE-2010-2237, CVE-2010-2238, CVE-2010-2239,
>> plus more recent variations of the theme that didn't get CVEs because
>> they were caught before they could hurt users).
>>
>> Disabling probing entirely is a (hamfisted) way to ensure you always
>> specify the format.
>>
>> Unfortunately, the new option is not available with -readconfig.
>> There's no obvious option group to take it. I think we could use a
>> "miscellaneous" option group.
>>
>> Signed-off-by: Markus Armbruster <arm...@redhat.com>
>> ---
>> block.c | 9 ++++++++-
>> include/block/block.h | 2 +-
>> qemu-options.hx | 12 ++++++++++++
>> vl.c | 6 +++++-
>> 4 files changed, 26 insertions(+), 3 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index 0fe97de..5865309 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -103,6 +103,7 @@ static void bdrv_reset_dirty(BlockDriverState *bs,
>> int64_t cur_sector,
>> int nr_sectors);
>> /* If non-zero, use only whitelisted block drivers */
>> static int use_bdrv_whitelist;
>> +static bool bdrv_image_probing_disabled;
>> #ifdef _WIN32
>> static int is_windows_drive_prefix(const char *filename)
>> @@ -751,6 +752,11 @@ static int find_image_format(BlockDriverState *bs,
>> const char *filename,
>> return ret;
>> }
>> + if (bdrv_image_probing_disabled) {
>> + error_setg(errp, "Format not specified and image probing disabled");
>> + return -EINVAL;
>> + }
>> +
>> ret = bdrv_pread(bs, 0, buf, sizeof(buf));
>> if (ret < 0) {
>> error_setg_errno(errp, -ret, "Could not read image for determining
>> its "
>> @@ -4909,9 +4915,10 @@ void bdrv_init(void)
>> module_call_init(MODULE_INIT_BLOCK);
>> }
>> -void bdrv_init_with_whitelist(void)
>> +void bdrv_init_with_whitelist(bool no_format_probing)
>> {
>> use_bdrv_whitelist = 1;
>> + bdrv_image_probing_disabled = no_format_probing;
>> bdrv_init();
>> }
>> diff --git a/include/block/block.h b/include/block/block.h
>> index 4c57d63..b5a8b23 100644
>> --- a/include/block/block.h
>> +++ b/include/block/block.h
>> @@ -177,7 +177,7 @@ void bdrv_io_limits_enable(BlockDriverState *bs);
>> void bdrv_io_limits_disable(BlockDriverState *bs);
>> void bdrv_init(void);
>> -void bdrv_init_with_whitelist(void);
>> +void bdrv_init_with_whitelist(bool no_format_probing);
>> BlockDriver *bdrv_find_protocol(const char *filename,
>> bool allow_protocol_prefix,
>> Error **errp);
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 319d971..8aa4d7b 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -963,6 +963,18 @@ STEXI
>> Disable SDL window close capability.
>> ETEXI
>> +DEF("no-format-probing", 0, QEMU_OPTION_no_format_probing,
>> + "-no-format-probing\n"
>> + " disable block image format probing\n", QEMU_ARCH_ALL)
>> +STEXI
>> +@item -no-format-probing
>> +@findex -no-format-probing
>> +Disable block image format probing. Probing is convenient, but
>> +probing untrusted raw images is insecure. To avoid it, always specify
>> +raw format explicitly. Disabling probing entirely is a (hamfisted)
>> +way to ensure you do.
>> +ETEXI
>> +
>> DEF("sdl", 0, QEMU_OPTION_sdl,
>> "-sdl enable SDL\n", QEMU_ARCH_ALL)
>> STEXI
>> diff --git a/vl.c b/vl.c
>> index 75ec292..94d5e15 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -2754,6 +2754,7 @@ int main(int argc, char **argv, char **envp)
>> #endif
>> bool defconfig = true;
>> bool userconfig = true;
>> + bool no_format_probing = false;
>> const char *log_mask = NULL;
>> const char *log_file = NULL;
>> GMemVTable mem_trace = {
>> @@ -2823,7 +2824,7 @@ int main(int argc, char **argv, char **envp)
>> nb_nics = 0;
>> - bdrv_init_with_whitelist();
>> + bdrv_init_with_whitelist(no_format_probing);
>> autostart = 1;
>> @@ -3381,6 +3382,9 @@ int main(int argc, char **argv, char **envp)
>> case QEMU_OPTION_no_quit:
>> no_quit = 1;
>> break;
>> + case QEMU_OPTION_no_format_probing:
>> + no_format_probing = true;
>> + break;
>> case QEMU_OPTION_sdl:
>> #ifdef CONFIG_SDL
>> display_type = DT_SDL;
>
> You're setting no_format_probing after you're using it, so it doesn't
> work very well. :-)
I really should not test stuff on a hurried Friday afternoon...
I'd appreciate opinions on whether this is wanted for 2.3. If it is,
I'll post a version that actually works.
