Xen devel - Mar 2013 - xen libxc - xen domain loader issues with handling large size domU ramdisk / kernel images

This is not directly related to xsa-25
(http://wiki.xen.org/wiki/Security_Announcements#XSA_25_Xen_domain_builder_Out-of-memory_due_to_malicious_kernel.2Framdisk),
but I found it while applying xsa-25 patch to xen. Please comment on
my suggestions and correct me if I am wrong.

file: xen-4.1.2/tools/libxc/xc_dom_core.c
function: xc_dom_check_gzip()
The function returns ''0'' if unzip length of the kernel /
ramdisk image
is invalid (greater than max allowed), pretending it as NOT gzipped
image. This causes the domain builder to proceed with large kernel /
ramdisk, which may cause dom0 going out-of-memory. Also one could make
a small gzip that unpacks to something big. So small gzipped file will
pass the size test and xen will try to load the kernel / ramdisk.
My suggestion here is to return ''unziplen'' if (unziplen >
max
allowed), as further checks
(xc_dom_kernel_check_size/xc_dom_ramdisk_check_size) are added by
xsa-25 which will make domain loader to quit before loading large
files. (please find the code below for reference)
Also I am not sure why size_t data type is checked for negative value.
The check  (unziplen < 0) will always return false as size_t is
unsigned.

size_t xc_dom_check_gzip(xc_interface *xch, void *blob, size_t ziplen)
{
    unsigned char *gzlen;
    size_t unziplen;

    if ( strncmp(blob, "\037\213", 2) )
        /* not gzipped */
        return 0;

    gzlen = blob + ziplen - 4;
    unziplen = gzlen[3] << 24 | gzlen[2] << 16 | gzlen[1] << 8
| gzlen[0];
    if ( (unziplen < 0) || (unziplen > XC_DOM_DECOMPRESS_MAX) )
    {
        xc_dom_printf
            (xch,
             "%s: size (zip %zd, unzip %zd) looks insane, skip
gunzip",
             __FUNCTION__, ziplen, unziplen);
        return 0; // instead of zero it should return
''unziplen''
    }

    return unziplen + 16;
}

The second point is related to xsa-25. After applying xsa-25 patch, in
function ''xc_dom_build_image()'' the code checks for ramdisk
file size,
and if the un-gzipped file size is invalid (greater than max allowed)
it assumes the unziplen = 0. It again pretends that the file is not
gzipped and proceeds to load domU. The domain loader proceeds further
with original ramdisk file size and it may make dom0 out-of-memory.
Please find the code below for reference.

int xc_dom_build_image(struct xc_dom_image *dom) {
...
...
        unziplen = xc_dom_check_gzip(dom->xch, dom->ramdisk_blob,
dom->ramdisk_size);
        if ( xc_dom_ramdisk_check_size(dom, unziplen) != 0 )
            unziplen = 0;  // here instead of unziplen = 0, it should
print error, cleanup and return (goto err;)

        ramdisklen = unziplen ? unziplen : dom->ramdisk_size;
....
....
}


Thank you.
~Shri

>>> On 21.03.13 at 00:33, Shriganesh Shintre
<shriganeshs@gmail.com> wrote:
> This is not directly related to xsa-25
>
(http://wiki.xen.org/wiki/Security_Announcements#XSA_25_Xen_domain_builder_O
> ut-of-memory_due_to_malicious_kernel.2Framdisk),
> but I found it while applying xsa-25 patch to xen. Please comment on
> my suggestions and correct me if I am wrong.
> 
> file: xen-4.1.2/tools/libxc/xc_dom_core.c
> function: xc_dom_check_gzip()
> The function returns ''0'' if unzip length of the kernel /
ramdisk image
> is invalid (greater than max allowed), pretending it as NOT gzipped
> image. This causes the domain builder to proceed with large kernel /
> ramdisk, which may cause dom0 going out-of-memory.
How? The (apparently or truly) uncompressed image is already in
memory at this point.
> Also one could make
> a small gzip that unpacks to something big. So small gzipped file will
> pass the size test and xen will try to load the kernel / ramdisk.
But you just above correctly said that it''s the decompressed
length that is being range checked.
> My suggestion here is to return ''unziplen'' if (unziplen
> max
> allowed), as further checks
> (xc_dom_kernel_check_size/xc_dom_ramdisk_check_size) are added by
> xsa-25 which will make domain loader to quit before loading large
> files. (please find the code below for reference)
> Also I am not sure why size_t data type is checked for negative value.
> The check  (unziplen < 0) will always return false as size_t is
> unsigned.
This is certainly bogus, but benign, as the check on the right hand
side of || will automatically take care of that situation.
> The second point is related to xsa-25. After applying xsa-25 patch, in
> function ''xc_dom_build_image()'' the code checks for
ramdisk file size,
> and if the un-gzipped file size is invalid (greater than max allowed)
> it assumes the unziplen = 0. It again pretends that the file is not
> gzipped and proceeds to load domU. The domain loader proceeds further
> with original ramdisk file size and it may make dom0 out-of-memory.
Once again - how? The image - again - is already in memory.

You may want to note that internally to libxc there''s no caller of
xc_linux_build_mem() (i.e. eventual range checks on the memory
blocks passed to this function would need to be done by the
caller anyway), and all other functions in xc_dom_compat_linux.c
end up using mmap() (i.e. don''t have real potential for driving
Dom0 out of memory).

Jan