diff options
author | Ian Jackson <ian.jackson@eu.citrix.com> | 2013-06-14 16:45:40 +0100 |
---|---|---|
committer | Ian Jackson <Ian.Jackson@eu.citrix.com> | 2013-06-14 16:45:40 +0100 |
commit | 968c0399159c65e24bb8b9969259e18791e1f4d8 (patch) | |
tree | bed44742e90828b7d4a6ad7924e1efd209f68b70 /tools | |
parent | 282188ea84b9e0f9c4865f0609e7740f2f28e7b0 (diff) | |
download | xen-968c0399159c65e24bb8b9969259e18791e1f4d8.tar.gz xen-968c0399159c65e24bb8b9969259e18791e1f4d8.tar.bz2 xen-968c0399159c65e24bb8b9969259e18791e1f4d8.zip |
libelf: check loops for running away
Ensure that libelf does not have any loops which can run away
indefinitely even if the input is bogus. (Grepped for \bfor, \bwhile
and \bgoto in libelf and xc_dom_*loader*.c.)
Changes needed:
* elf_note_next uses the note's unchecked alleged length, which might
wrap round. If it does, return ELF_MAX_PTRVAL (0xfff..fff) instead,
which will be beyond the end of the section and so terminate the
caller's loop.
* In various loops over section and program headers, check that the
calculated header pointer is still within the image, and quit the
loop if it isn't.
We have not changed loops which might, in principle, iterate over the
whole image - even if they might do so one byte at a time with a
nontrivial access check function in the middle.
This is part of the fix to a security issue, XSA-55.
Conflicts in Xen 4.1 version of the series:
* Trivial conflict due to elf_note_numeric_array not existing.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Diffstat (limited to 'tools')
-rw-r--r-- | tools/libxc/xc_dom_elfloader.c | 33 |
1 files changed, 24 insertions, 9 deletions
diff --git a/tools/libxc/xc_dom_elfloader.c b/tools/libxc/xc_dom_elfloader.c index 77b2e5b8d2..8e0d9d0f26 100644 --- a/tools/libxc/xc_dom_elfloader.c +++ b/tools/libxc/xc_dom_elfloader.c @@ -28,6 +28,7 @@ #include "xg_private.h" #include "xc_dom.h" +#include "xc_bitops.h" #define XEN_VER "xen-3.0" @@ -120,6 +121,7 @@ static elf_errorstatus xc_dom_load_elf_symtab(struct xc_dom_image *dom, ELF_PTRVAL_CHAR hdr; size_t size; unsigned h, count, type, i, tables = 0; + unsigned long *strtab_referenced = NULL; if ( elf_swap(elf) ) { @@ -220,22 +222,35 @@ static elf_errorstatus xc_dom_load_elf_symtab(struct xc_dom_image *dom, symtab, maxaddr); count = elf_shdr_count(&syms); + /* elf_shdr_count guarantees that count is reasonable */ + + strtab_referenced = xc_dom_malloc(dom, bitmap_size(count)); + if ( strtab_referenced == NULL ) + return -1; + bitmap_clear(strtab_referenced, count); + /* Note the symtabs @h linked to by any strtab @i. */ + for ( i = 0; i < count; i++ ) + { + shdr2 = elf_shdr_by_index(&syms, i); + if ( elf_uval(&syms, shdr2, sh_type) == SHT_SYMTAB ) + { + h = elf_uval(&syms, shdr2, sh_link); + if (h < count) + set_bit(h, strtab_referenced); + } + } + for ( h = 0; h < count; h++ ) { shdr = ELF_OBSOLETE_VOIDP_CAST elf_shdr_by_index(&syms, h); + if ( !elf_access_ok(elf, ELF_HANDLE_PTRVAL(shdr), 1) ) + /* input has an insane section header count field */ + break; type = elf_uval(&syms, shdr, sh_type); if ( type == SHT_STRTAB ) { - /* Look for a strtab @i linked to symtab @h. */ - for ( i = 0; i < count; i++ ) - { - shdr2 = elf_shdr_by_index(&syms, i); - if ( (elf_uval(&syms, shdr2, sh_type) == SHT_SYMTAB) && - (elf_uval(&syms, shdr2, sh_link) == h) ) - break; - } /* Skip symtab @h if we found no corresponding strtab @i. */ - if ( i == count ) + if ( !test_bit(h, strtab_referenced) ) { if ( elf_64bit(&syms) ) elf_store_field(elf, shdr, e64.sh_offset, 0); |