[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 18/22] libxc: Add range checking to xc_dom_binloader
On 07/06/13 19:27, Ian Jackson wrote: > 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> > > 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; More void pointer arithmetic. If I am reading the above correctly, it appears to be a glorified probe_end = dom->kernel_blob + (uintptr_t)min(dom->kernel_size - sizeof(*table), 8192); which looks to be a more simple representation? ~Andrew > > 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); > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |