qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] Multi potential integer overflow vulnerabilities
@ 2008-08-25  3:32 wangtielei
  0 siblings, 0 replies; 2+ messages in thread
From: wangtielei @ 2008-08-25  3:32 UTC (permalink / raw)
  To: qemu-devel

[-- Attachment #1: Type: text/plain, Size: 5312 bytes --]

Hi, everyboy,
 
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, 
so 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, thinks!

Wangtielei
wangtielei (AT) icst (dot) pku (dot) edu (dot) cn

[-- Attachment #2: Type: text/html, Size: 34325 bytes --]

^ permalink raw reply	[flat|nested] 2+ messages in thread

* [Qemu-devel] multi potential integer overflow vulnerabilities
@ 2008-08-25 11:09 tielei wang
  0 siblings, 0 replies; 2+ messages in thread
From: tielei wang @ 2008-08-25 11:09 UTC (permalink / raw)
  To: qemu-devel

[-- Attachment #1: Type: text/plain, Size: 5256 bytes --]

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

[-- Attachment #2: Type: text/html, Size: 22170 bytes --]

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2008-08-25 11:55 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-08-25 11:09 [Qemu-devel] multi potential integer overflow vulnerabilities tielei wang
  -- strict thread matches above, loose matches on Subject: below --
2008-08-25  3:32 [Qemu-devel] Multi " wangtielei

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).