[Qemu-devel] multi potential integer overflow vulnerabilities

[tielei wang]

Hi,

 

I think there are multi- potential integer overflow vulnerabilities in QEMU

 

The 1st one, boch_open() function in block-boch.c

 

diff -U 25 block-bochs.c block-bochs_patched.c
--- block-bochs.c       2008-01-07 03:38:41.000000000 +0800
+++ block-bochs_patched.c       2008-08-25 09:46:59.000000000 +0800
@@ -126,50 +126,52 @@
     s->fd = fd;
     if (read(fd, &bochs, sizeof(bochs)) != sizeof(bochs)) {
         goto fail;
     }
// .......
     lseek(s->fd, le32_to_cpu(bochs.header), SEEK_SET);
 
     s->catalog_size = le32_to_cpu(bochs.extra.redolog.catalog);
+    if(s->catalog_size > MAX/4)
+        goto fail;
     s->catalog_bitmap = qemu_malloc(s->catalog_size * 4);

 

boch_open() reads some datas from "fd" to "bochs", so "bochs.extra.redolog.catalog" is a tainted value. s->catalog_size * 4 is a potential integer overflow operation, results in s->catalog_bitmap is allocated with a small memory region.

 

The 2nd one, cloop_open() function in block-cloop.c

 

diff -U 6 block-cloop.c block-cloop_pathched.c
--- block-cloop.c       2008-01-07 03:38:41.000000000 +0800
+++ block-cloop_pathched.c      2008-08-25 10:01:23.000000000 +0800
@@ -72,12 +72,17 @@
     if(read(s->fd,&s->n_blocks,4)<4)
        goto cloop_close;
     s->n_blocks=be32_to_cpu(s->n_blocks);
 
     /* read offsets */
     offsets_size=s->n_blocks*sizeof(uint64_t);
+    if (s->n_blocks > MAX/ sizeof(uint64_t)){
+        close(s->fd);
+        return -1;
+    }
+  
     if(!(s->offsets=(uint64_t*)malloc(offsets_size))

 

s->n_blocks is a tainted data from file, so "offsets_size=s->n_blocks*sizeof(uint64_t) " is a potential integer overflow operation. offsets_size is smaller than the value it should be.

 

The 3rd one, parallels_open() function in block- parallels.c

 

diff -U 16 block-parallels.c block-parallels_patched.c
--- block-parallels.c   2008-01-07 03:38:41.000000000 +0800
+++ block-parallels_patched.c   2008-08-25 10:10:35.000000000 +0800
@@ -87,32 +87,34 @@
     if (read(fd, &ph, sizeof(ph)) != sizeof(ph))
         goto fail;

//……
     s->tracks = le32_to_cpu(ph.tracks);
     s->catalog_size = le32_to_cpu(ph.catalog_entries);
+    if(s->catalog_size > MAX/4)
+       goto fail;
       s->catalog_bitmap = qemu_malloc(s->catalog_size * 4);

Similar with the 1st one, ph.catalog_entries is a tainted data. We can't use it as malloc function's parameter.

 

The 4th one, qcow_open() function in block-qcow.c

 

    if (bdrv_pread(s->hd, 0, &header, sizeof(header)) != sizeof(header))
        goto fail;
 //......
    s->cluster_bits = header.cluster_bits;
    s->cluster_size = 1 << s->cluster_bits;
    s->cluster_sectors = 1 << (s->cluster_bits - 9);
    s->l2_bits = header.l2_bits;
    s->l2_size = 1 << s->l2_bits;
    bs->total_sectors = header.size / 512;
    s->cluster_offset_mask = (1LL << (63 - s->cluster_bits)) - 1;

 

    /* read the level 1 table */
    shift = s->cluster_bits + s->l2_bits;
    s->l1_size = (header.size + (1LL << shift) - 1) >> shift;

 

    s->l1_table_offset = header.l1_table_offset;
    s->l1_table = qemu_malloc(s->l1_size * sizeof(uint64_t));

 

A crafted file could cause s->l1_size big enough so that "s->l1_size * sizeof(uint64_t)" is an integer overflow operation.

By the way, qcow_open() function has other similar problems.

 

The 5th one, vmdk_open() function in block-vmdk.c

 

                VMDK4Header header;

                if (read(fd, &header, sizeof(header)) != sizeof(header))

                        goto fail;

                s->size = le32_to_cpu(header.capacity);

                prv->cluster_sectors = le32_to_cpu(header.granularity);

                prv->l2_size = le32_to_cpu(header.num_gtes_per_gte);

                prv->l1_entry_sectors = prv->l2_size * prv->cluster_sectors;

                if (prv->l1_entry_sectors <= 0)

                        goto fail;

                prv->l1_size = (s->size + prv->l1_entry_sectors - 1)

                               / prv->l1_entry_sectors;

                prv->l1_table_offset = le64_to_cpu(header.rgd_offset) << 9;

                prv->l1_backup_table_offset =

                        le64_to_cpu(header.gd_offset) << 9;

        } else {

                goto fail;

        }

        /* read the L1 table */

+        if(prv->l1_size > INT_MAX/sizeof(uint32_t))

+            goto fail;

        l1_size = prv->l1_size * sizeof(uint32_t);

        prv->l1_table = malloc(l1_size);

 

If header.capacity is very huge, but both header.granularity and header.num_gtes_per_gte are 1, then

prv->l1_size = (s->size +prv->l1_entry_sectors - 1)/ prv->l1_entry_sectors = s->size = header.capacity.

Now, prv->l1_size * sizeof(uint32_t) is an integer overflow operation.

 

The 6th one, vpc_open() function in block-vpc.c

if (read(fd, &header, HEADER_SIZE) != HEADER_SIZE)
        goto fail;

//......

s->pagetable_entries = be32_to_cpu(header.type.sparse.pagetable_entries);
    s->pagetable = qemu_malloc(s->pagetable_entries * 4);

header.type.sparse.pagetable_entries is a tainted value, so it shouldn't be used as malloc's parameter directlty after multiplication operation.

 

Waiting for your reply, thanks!

(I sent a mail before this one, but the mail didn't appear in the mail list)

Wangtielei