All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ian Jackson <ian.jackson@eu.citrix.com>
To: xen-devel@lists.xensource.com
Cc: andrew.cooper3@citrix.com, mattjd@gmail.com,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	security@xen.org
Subject: [PATCH 18/22] libxc: Add range checking to xc_dom_binloader
Date: Tue, 11 Jun 2013 19:21:01 +0100	[thread overview]
Message-ID: <1370974865-19554-19-git-send-email-ian.jackson@eu.citrix.com> (raw)
In-Reply-To: <1370974865-19554-1-git-send-email-ian.jackson@eu.citrix.com>

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@eu.citrix.com>

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

  parent reply	other threads:[~2013-06-11 18:21 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-11 18:20 [PATCH v7 00/22] XSA55 libelf fixes for unstable Ian Jackson
2013-06-11 18:20 ` [PATCH 01/22] libelf: abolish libelf-relocate.c Ian Jackson
2013-06-11 18:20 ` [PATCH 02/22] libxc: introduce xc_dom_seg_to_ptr_pages Ian Jackson
2013-06-11 18:44   ` Andrew Cooper
2013-06-11 18:20 ` [PATCH 03/22] libxc: Fix range checking in xc_dom_pfn_to_ptr etc Ian Jackson
2013-06-11 19:01   ` Andrew Cooper
2013-06-11 18:20 ` [PATCH 04/22] libelf: add `struct elf_binary*' parameter to elf_load_image Ian Jackson
2013-06-11 18:20 ` [PATCH 05/22] libelf: abolish elf_sval and elf_access_signed Ian Jackson
2013-06-11 18:20 ` [PATCH 06/22] libelf: move include of <asm/guest_access.h> to top of file Ian Jackson
2013-06-11 18:20 ` [PATCH 07/22] libelf/xc_dom_load_elf_symtab: Do not use "syms" uninitialised Ian Jackson
2013-06-11 18:20 ` [PATCH 08/22] libelf: introduce macros for memory access and pointer handling Ian Jackson
2013-06-11 18:20 ` [PATCH 09/22] tools/xcutils/readnotes: adjust print_l1_mfn_valid_note Ian Jackson
2013-06-11 18:20 ` [PATCH 10/22] libelf: check nul-terminated strings properly Ian Jackson
2013-06-11 19:14   ` Andrew Cooper
2013-06-11 18:20 ` [PATCH 11/22] libelf: check all pointer accesses Ian Jackson
2013-06-11 19:19   ` Andrew Cooper
2013-06-11 18:20 ` [PATCH 12/22] libelf: Check pointer references in elf_is_elfbinary Ian Jackson
2013-06-11 19:03   ` Andrew Cooper
2013-06-11 18:20 ` [PATCH 13/22] libelf: Make all callers call elf_check_broken Ian Jackson
2013-06-11 18:20 ` [PATCH 14/22] libelf: use C99 bool for booleans Ian Jackson
2013-06-11 19:04   ` Andrew Cooper
2013-06-11 18:20 ` [PATCH 15/22] libelf: use only unsigned integers Ian Jackson
2013-06-11 19:22   ` Andrew Cooper
2013-06-11 18:20 ` [PATCH 16/22] libelf: check loops for running away Ian Jackson
2013-06-11 19:28   ` Andrew Cooper
2013-06-11 18:21 ` [PATCH 17/22] libelf: abolish obsolete macros Ian Jackson
2013-06-11 18:21 ` Ian Jackson [this message]
2013-06-11 19:11   ` [PATCH 18/22] libxc: Add range checking to xc_dom_binloader Andrew Cooper
2013-06-11 18:21 ` [PATCH 19/22] libxc: check failure of xc_dom_*_to_ptr, xc_map_foreign_range Ian Jackson
2013-06-11 19:10   ` Andrew Cooper
2013-06-11 18:21 ` [PATCH 20/22] libxc: check return values from malloc Ian Jackson
2013-06-11 19:05   ` Andrew Cooper
2013-06-11 18:21 ` [PATCH 21/22] libxc: range checks in xc_dom_p2m_host and _guest Ian Jackson
2013-06-11 19:06   ` Andrew Cooper
2013-06-12 16:02   ` George Dunlap
2013-06-12 16:06     ` Ian Jackson
2013-06-12 16:08       ` George Dunlap
2013-06-12 18:26       ` Tim Deegan
2013-06-11 18:21 ` [PATCH 22/22] libxc: check blob size before proceeding in xc_dom_check_gzip Ian Jackson
2013-06-11 19:08   ` Andrew Cooper
2013-06-11 19:30 ` [PATCH v7 00/22] XSA55 libelf fixes for unstable Andrew Cooper
2013-06-12 13:45   ` Ian Jackson
2013-06-12 14:02     ` Ian Jackson
2013-06-12 14:19     ` Andrew Cooper
  -- strict thread matches above, loose matches on Subject: below --
2013-06-11 18:22 [PATCH v7 00/22] XSA55 libelf fixes for Xen 4.2 Ian Jackson
2013-06-11 18:22 ` [PATCH 18/22] libxc: Add range checking to xc_dom_binloader Ian Jackson
2013-06-07 18:35 [PATCH v6 00/22] XSA55 libelf fixes for Xen 4.2 Ian Jackson
2013-06-07 18:35 ` [PATCH 18/22] libxc: Add range checking to xc_dom_binloader Ian Jackson
2013-06-07 18:27 [PATCH v6 00/22] XSA55 libelf fixes for unstable Ian Jackson
2013-06-07 18:27 ` [PATCH 18/22] libxc: Add range checking to xc_dom_binloader Ian Jackson
2013-06-10 21:37   ` Andrew Cooper
2013-06-11 14:46     ` Ian Jackson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1370974865-19554-19-git-send-email-ian.jackson@eu.citrix.com \
    --to=ian.jackson@eu.citrix.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=mattjd@gmail.com \
    --cc=security@xen.org \
    --cc=xen-devel@lists.xensource.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.