libxc: builder: limit maximum size of kernel/ramdisk.

Allowing user supplied kernels of arbitrary sizes, especially during
decompression, can swallow up dom0 memory leading to either virtual
address space exhaustion in the builder process or allocation
failures/OOM killing of both toolstack and unrelated processes.

We disable these checks when building in a stub domain for pvgrub
since this uses the guest's own memory and is isolated.

Decompression of gzip compressed kernels and ramdisks has been safe
since 14954:58205257517d (Xen 3.1.0 onwards).

This is XSA-25 / CVE-2012-4544.

Also make explicit checks for buffer overflows in various
decompression routines. These were already ruled out due to other
properties of the code but check them as a belt-and-braces measure.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

1 files changed, 55 insertions, 4 deletions

diff --git a/tools/libxc/xc_dom_bzimageloader.c b/tools/libxc/xc_dom_bzimageloader.c
index 113d40ff2f..b1b2eb0e59 100644
--- a/tools/libxc/xc_dom_bzimageloader.c
+++ b/tools/libxc/xc_dom_bzimageloader.c
@@ -47,13 +47,19 @@ static int xc_try_bzip2_decode(
     char *out_buf;
     char *tmp_buf;
     int retval = -1;
-    int outsize;
+    unsigned int outsize;
     uint64_t total;
 
     stream.bzalloc = NULL;
     stream.bzfree = NULL;
     stream.opaque = NULL;
 
+    if ( dom->kernel_size == 0)
+    {
+        DOMPRINTF("BZIP2: Input is 0 size");
+        return -1;
+    }
+
     ret = BZ2_bzDecompressInit(&stream, 0, 0);
     if ( ret != BZ_OK )
     {
@@ -66,6 +72,17 @@ static int xc_try_bzip2_decode(
      * the input buffer to start, and we'll realloc as needed.
      */
     outsize = dom->kernel_size;
+
+    /*
+     * stream.avail_in and outsize are unsigned int, while kernel_size
+     * is a size_t. Check we aren't overflowing.
+     */
+    if ( outsize != dom->kernel_size )
+    {
+        DOMPRINTF("BZIP2: Input too large");
+        goto bzip2_cleanup;
+    }
+
     out_buf = malloc(outsize);
     if ( out_buf == NULL )
     {
@@ -98,13 +115,20 @@ static int xc_try_bzip2_decode(
         if ( stream.avail_out == 0 )
         {
             /* Protect against output buffer overflow */
-            if ( outsize > INT_MAX / 2 )
+            if ( outsize > UINT_MAX / 2 )
             {
                 DOMPRINTF("BZIP2: output buffer overflow");
                 free(out_buf);
                 goto bzip2_cleanup;
             }
 
+            if ( xc_dom_kernel_check_size(dom, outsize * 2) )
+            {
+                DOMPRINTF("BZIP2: output too large");
+                free(out_buf);
+                goto bzip2_cleanup;
+            }
+
             tmp_buf = realloc(out_buf, outsize * 2);
             if ( tmp_buf == NULL )
             {
@@ -172,9 +196,15 @@ static int _xc_try_lzma_decode(
     unsigned char *out_buf;
     unsigned char *tmp_buf;
     int retval = -1;
-    int outsize;
+    size_t outsize;
     const char *msg;
 
+    if ( dom->kernel_size == 0)
+    {
+        DOMPRINTF("%s: Input is 0 size", what);
+        return -1;
+    }
+
     /* sigh.  We don't know up-front how much memory we are going to need
      * for the output buffer.  Allocate the output buffer to be equal
      * the input buffer to start, and we'll realloc as needed.
@@ -244,13 +274,20 @@ static int _xc_try_lzma_decode(
         if ( stream->avail_out == 0 )
         {
             /* Protect against output buffer overflow */
-            if ( outsize > INT_MAX / 2 )
+            if ( outsize > SIZE_MAX / 2 )
             {
                 DOMPRINTF("%s: output buffer overflow", what);
                 free(out_buf);
                 goto lzma_cleanup;
             }
 
+            if ( xc_dom_kernel_check_size(dom, outsize * 2) )
+            {
+                DOMPRINTF("%s: output too large", what);
+                free(out_buf);
+                goto lzma_cleanup;
+            }
+
             tmp_buf = realloc(out_buf, outsize * 2);
             if ( tmp_buf == NULL )
             {
@@ -359,6 +396,12 @@ static int xc_try_lzo1x_decode(
         0x89, 0x4c, 0x5a, 0x4f, 0x00, 0x0d, 0x0a, 0x1a, 0x0a
     };
 
+    /*
+     * lzo_uint should match size_t. Check that this is the case to be
+     * sure we won't overflow various lzo_uint fields.
+     */
+    XC_BUILD_BUG_ON(sizeof(lzo_uint) != sizeof(size_t));
+
     ret = lzo_init();
     if ( ret != LZO_E_OK )
     {
@@ -438,6 +481,14 @@ static int xc_try_lzo1x_decode(
         if ( src_len <= 0 || src_len > dst_len || src_len > left )
             break;
 
+        msg = "Output buffer overflow";
+        if ( *size > SIZE_MAX - dst_len )
+            break;
+
+        msg = "Decompressed image too large";
+        if ( xc_dom_kernel_check_size(dom, *size + dst_len) )
+            break;
+
         msg = "Failed to (re)alloc memory";
         tmp_buf = realloc(out_buf, *size + dst_len);
         if ( tmp_buf == NULL )