[Xen-devel] [PATCH 18/23] libxc: Add range checking to xc_dom_binloader

[Ian Jackson <ian.jackson@xxxxxxxxxxxxx>]

This is a simple binary image loader with its own metadata format.
However, it is too careless with image-supplied values.

Add the following checks:

 * That the image is bigger than the metadata table; otherwise the
   pointer arithmetic to calculate the metadata table location may
   yield undefined and dangerous values.

 * When clamping the end of the region to search, that we do not
   calculate pointers beyond the end of the image.  The C
   specification does not permit this and compilers are becoming ever
   more determined to miscompile code when they can "prove" various
   falsehoods based on assertions from the C spec.

 * That the supplied image is big enough for the text we are allegedly
   copying from it.  Otherwise we might have a read overrun and copy
   the results (perhaps a lot of secret data) into the guest.

This is part of the fix to a security issue, XSA-55.

Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

v6: Add a missing `return -EINVAL' (Matthew Daley).
    Fix an error in the commit message (Matthew Daley).

v5: This patch is new in this version of the series.
---
 tools/libxc/xc_dom_binloader.c |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/tools/libxc/xc_dom_binloader.c b/tools/libxc/xc_dom_binloader.c
index d2de04c..7eaf071 100644
--- a/tools/libxc/xc_dom_binloader.c
+++ b/tools/libxc/xc_dom_binloader.c
@@ -123,9 +123,12 @@ static struct xen_bin_image_table *find_table(struct 
xc_dom_image *dom)
     uint32_t *probe_ptr;
     uint32_t *probe_end;
 
+    if ( dom->kernel_size < sizeof(*table) )
+        return NULL;
     probe_ptr = dom->kernel_blob;
     probe_end = dom->kernel_blob + dom->kernel_size - sizeof(*table);
-    if ( (void*)probe_end > (dom->kernel_blob + 8192) )
+    if ( dom->kernel_size >= 8192 &&
+         (void*)probe_end > (dom->kernel_blob + 8192) )
         probe_end = dom->kernel_blob + 8192;
 
     for ( table = NULL; probe_ptr < probe_end; probe_ptr++ )
@@ -282,6 +285,14 @@ static int xc_dom_load_bin_kernel(struct xc_dom_image *dom)
         return -EINVAL;
     }
 
+    if ( image_size < skip ||
+         image_size - skip < text_size )
+    {
+        DOMPRINTF("%s: image is too small for declared text size",
+                  __FUNCTION__);
+        return -EINVAL;
+    }
+
     memcpy(dest, image + skip, text_size);
     memset(dest + text_size, 0, bss_size);
 
-- 
1.7.2.5


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel