[Xen-bugs] [Bug 1330] New: Potential integer overflow bug in tdvmdk_open() in ./tools/blktap/drivers/block-vmdk.c

[bugzilla-daemon@xxxxxxxxxxxxxxxxxxx]

http://bugzilla.xensource.com/bugzilla/show_bug.cgi?id=1330

           Summary: Potential integer overflow bug in tdvmdk_open() in
                    ./tools/blktap/drivers/block-vmdk.c
           Product: Xen
           Version: unspecified
          Platform: Unspecified
        OS/Version: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Tools
        AssignedTo: xen-bugs@xxxxxxxxxxxxxxxxxxx
        ReportedBy: wangtielei@xxxxxxxxxxxxxxx


I think there is a potential integer overflow bug in in tdvmdk_open() in
./tools/blktap/drivers/block-vmdk.c. 
tdvmdk_open() opens the disk file and initializes aio state, however,
tdvmdk_open doesn't check input file rightly. A crafted input file could cause
integer overflow and heap crash.
Look below code, an input file with a malformed VMDK4Header could cause integer
overflow.
diff -U 20 block-vmdk.c block-vmdk_patched.c 
--- block-vmdk.c        2008-04-25 21:03:45.000000000 +0800
+++ block-vmdk_patched.c        2008-08-22 15:32:15.000000000 +0800
@@ -175,40 +175,42 @@
        } else if (magic == VMDK4_MAGIC) {
                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, however,
the result is used in malloc function, right?
Waiting for your reply, thinks!


-- 
Configure bugmail: 
http://bugzilla.xensource.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

_______________________________________________
Xen-bugs mailing list
Xen-bugs@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-bugs