[Qemu-devel] [PATCH] qcow2: Reject unrealistically large header extensions

[Qemu-devel] [PATCH] qcow2: Reject unrealistically large header extensions

[Kevin Wolf]

Image files that make qemu-img info read several gigabytes into the
unknown header extensions list are bad. Just fail opening the image
if an extension claims to be large.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index f68f0e1..077fe05 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -108,6 +108,11 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
 #ifdef DEBUG_EXT
         printf("ext.magic = 0x%x\n", ext.magic);
 #endif
+        if (ext.len > 65536) {
+            error_report("Header extension larger than 64k - this looks wrong");
+            return -ENOTSUP;
+        }
+
         switch (ext.magic) {
         case QCOW2_EXT_MAGIC_END:
             return 0;
-- 
1.7.6.5

Re: [Qemu-devel] [PATCH] qcow2: Reject unrealistically large header extensions

[Stefan Hajnoczi]

On Mon, Feb 27, 2012 at 4:27 PM, Kevin Wolf <kwolf@redhat.com> wrote:
> +        if (ext.len > 65536) {
> +            error_report("Header extension larger than 64k - this looks wrong");
> +            return -ENOTSUP;
> +        }

This is an implementation limit and not in the spec, but I think it's
reasonable.

Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

Re: [Qemu-devel] [PATCH] qcow2: Reject unrealistically large header extensions

[Kevin Wolf]

Am 28.02.2012 10:33, schrieb Stefan Hajnoczi:
> On Mon, Feb 27, 2012 at 4:27 PM, Kevin Wolf <kwolf@redhat.com> wrote:
>> +        if (ext.len > 65536) {
>> +            error_report("Header extension larger than 64k - this looks wrong");
>> +            return -ENOTSUP;
>> +        }
> 
> This is an implementation limit and not in the spec, but I think it's
> reasonable.
> 
> Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>

Hm, actually, now that I look at this patch again, I think there's a
much better error condition that even matches the spec:

    if (offset + ext.len > end_offset)

I'll send a changed version of the patch.

Kevin

Re: [Qemu-devel] [PATCH] qcow2: Reject unrealistically large header extensions

[Stefan Hajnoczi]

On Tue, Feb 28, 2012 at 9:47 AM, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 28.02.2012 10:33, schrieb Stefan Hajnoczi:
>> On Mon, Feb 27, 2012 at 4:27 PM, Kevin Wolf <kwolf@redhat.com> wrote:
>>> +        if (ext.len > 65536) {
>>> +            error_report("Header extension larger than 64k - this looks wrong");
>>> +            return -ENOTSUP;
>>> +        }
>>
>> This is an implementation limit and not in the spec, but I think it's
>> reasonable.
>>
>> Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
>
> Hm, actually, now that I look at this patch again, I think there's a
> much better error condition that even matches the spec:
>
>    if (offset + ext.len > end_offset)

Careful, integer overflow.

Stefan

Re: [Qemu-devel] [PATCH] qcow2: Reject unrealistically large header extensions

[Kevin Wolf]

Am 28.02.2012 11:00, schrieb Stefan Hajnoczi:
> On Tue, Feb 28, 2012 at 9:47 AM, Kevin Wolf <kwolf@redhat.com> wrote:
>> Am 28.02.2012 10:33, schrieb Stefan Hajnoczi:
>>> On Mon, Feb 27, 2012 at 4:27 PM, Kevin Wolf <kwolf@redhat.com> wrote:
>>>> +        if (ext.len > 65536) {
>>>> +            error_report("Header extension larger than 64k - this looks wrong");
>>>> +            return -ENOTSUP;
>>>> +        }
>>>
>>> This is an implementation limit and not in the spec, but I think it's
>>> reasonable.
>>>
>>> Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
>>
>> Hm, actually, now that I look at this patch again, I think there's a
>> much better error condition that even matches the spec:
>>
>>    if (offset + ext.len > end_offset)
> 
> Careful, integer overflow.

offset/end_offset are uint64_t offsets into the first cluster, ext.len
is uint32_t. Looks safe.

Kevin