[Xen-devel] [PATCH 1/8] libelf: loop safety: Introduce elf_iter_ok and elf_strcmp_safe

[Ian Jackson <ian.jackson@xxxxxxxxxxxxx>]

This will allow us to keep track of the total amount of work we are
doing.  When it becomes excessive, we mark the ELF broken, and stop
processing.

This is a more robust way of preventing DoS problems by bad images
than attempting to prove, for each of the (sometimes rather deeply
nested) loops, that the total work is "reasonable".  We bound the
notional work by 4x the image size (plus 1M).

Also introduce elf_strcmp_safe, which unconditionally does the work,
but increments the count so any outer loop may be aborted if
necessary.

Currently there are no callers, so no functional change.

Signed-off-by: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
---
 xen/common/libelf/libelf-loader.c | 14 ++++++++++++++
 xen/include/xen/libelf.h          | 21 +++++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/xen/common/libelf/libelf-loader.c 
b/xen/common/libelf/libelf-loader.c
index a72cd8a..00479af 100644
--- a/xen/common/libelf/libelf-loader.c
+++ b/xen/common/libelf/libelf-loader.c
@@ -38,6 +38,7 @@ elf_errorstatus elf_init(struct elf_binary *elf, const char 
*image_input, size_t
     ELF_HANDLE_DECL(elf_shdr) shdr;
     unsigned i, count, section, link;
     uint64_t offset;
+    const uint64_t max_size_for_deacc = (1UL << 63)/ELF_MAX_ITERATION_FACTOR;
 
     if ( !elf_is_elfbinary(image_input, size) )
     {
@@ -52,6 +53,10 @@ elf_errorstatus elf_init(struct elf_binary *elf, const char 
*image_input, size_t
     elf->class = elf_uval_3264(elf, elf->ehdr, e32.e_ident[EI_CLASS]);
     elf->data = elf_uval_3264(elf, elf->ehdr, e32.e_ident[EI_DATA]);
 
+    elf->iteration_deaccumulator = 1024*1024 +
+        (size > max_size_for_deacc ? max_size_for_deacc : size)
+        * ELF_MAX_ITERATION_FACTOR;        
+
     /* Sanity check phdr. */
     offset = elf_uval(elf, elf->ehdr, e_phoff) +
         elf_uval(elf, elf->ehdr, e_phentsize) * elf_phdr_count(elf);
@@ -546,6 +551,15 @@ uint64_t elf_lookup_addr(struct elf_binary * elf, const 
char *symbol)
     return value;
 }
 
+bool elf_iter_ok_counted(struct elf_binary *elf, uint64_t maxcopysz) {
+    if (maxcopysz > elf->iteration_deaccumulator)
+        elf_mark_broken(elf, "excessive iteration - too much work to parse");
+    if (elf->broken)
+        return false;
+    elf->iteration_deaccumulator -= maxcopysz;
+    return true;
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/include/xen/libelf.h b/xen/include/xen/libelf.h
index 1b763f3..294231a 100644
--- a/xen/include/xen/libelf.h
+++ b/xen/include/xen/libelf.h
@@ -56,6 +56,8 @@ typedef void elf_log_callback(struct elf_binary*, void 
*caller_data,
 #define ELF_MAX_STRING_LENGTH 4096
 #define ELF_MAX_TOTAL_NOTE_COUNT 65536
 
+#define ELF_MAX_ITERATION_FACTOR 4
+
 /* ------------------------------------------------------------------------ */
 
 /* Macros for accessing the input image and output area. */
@@ -201,6 +203,9 @@ struct elf_binary {
     uint64_t bsd_symtab_pstart;
     uint64_t bsd_symtab_pend;
 
+    /* private */
+    uint64_t iteration_deaccumulator;
+    
     /*
      * caller's other acceptable destination.
      * Set by elf_set_xdest.  Do not set these directly.
@@ -264,6 +269,14 @@ uint64_t elf_access_unsigned(struct elf_binary *elf, 
elf_ptrval ptr,
 
 uint64_t elf_round_up(struct elf_binary *elf, uint64_t addr);
 
+bool elf_iter_ok_counted(struct elf_binary *elf, uint64_t count);
+  /* It is OK for count to be out by a smallish constant factor.
+   * It is OK for count to be 0, as we clamp it to 1, so we
+   * can use lengths or sizes from the image. */
+
+static inline bool elf_iter_ok(struct elf_binary *elf)
+    { return elf_iter_ok_counted(elf,1); }
+
 const char *elf_strval(struct elf_binary *elf, elf_ptrval start);
   /* may return NULL if the string is out of range etc. */
 
@@ -463,6 +476,14 @@ static inline void *elf_memset_unchecked(void *s, int c, 
size_t n)
    * memcpy, memset and memmove to undefined MISTAKE things.
    */
 
+static inline int elf_strcmp_safe(struct elf_binary *elf,
+                                  const char *a, const char *b) {
+    elf_iter_ok_counted(elf, strlen(b));
+    return strcmp(a,b);
+}
+  /* Unlike other *_safe functions, elf_strcmp_safe is called on
+   * values already extracted from the image (eg by elf_strval),
+   * and fixed constant strings (typically, the latter is "b"). */
 
 /* Advances past amount bytes of the current destination area. */
 static inline void ELF_ADVANCE_DEST(struct elf_binary *elf, uint64_t amount)
-- 
2.1.4


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