Signed-off-by: Ian Jackson <ian.jack...@eu.citrix.com>
---
xen/include/xen/libelf.h | 92 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 92 insertions(+)
diff --git a/xen/include/xen/libelf.h b/xen/include/xen/libelf.h
index 6436bd7..8b75242 100644
--- a/xen/include/xen/libelf.h
+++ b/xen/include/xen/libelf.h
@@ -60,6 +60,96 @@ typedef void elf_log_callback(struct elf_binary*, void
*caller_data,
/* ------------------------------------------------------------------------ */
+/*
+ * DESIGN PRINCIPLES FOR THE SAFETY OF LIBELF
+ *
+ * libelf is a complex piece of code on a security boundary: when
+ * built as part of the tools, it parses guest kernels and loads them
+ * into guest memory. Bugs in libelf can become privilege escalation
+ * or denial of service bugs in the toolstack.
+ *
+ * We try to reduce the risk of such bugs by writing the actual format
+ * parsing in a mostly-safe subset of C. To avoid nonlocal exits or
+ * the need for explicit error-checking code, we make all references
+ * into the input image, or into guest memory, via an inherently safe
+ * wrapper system.
+ *
+ * This means that it is safe to simply honour the instructions from
+ * the image, even if they are nonsense. If the image implies wild
+ * pointer accesses, these will be harmlessly defused; a note will be
+ * made that things are broken; and processing can safely continue
+ * despite some of the operations having not been done. Eventually
+ * the error will be reported.
+ *
+ *
+ * To preserve these safety properties, there are some rules that
+ * programmers editing libelf need to follow:
+ *
+ * - Any loop needs to be accompanied by calls to elf_iter_ok (or
+ * elf_iter_ok_counted).
+ *
+ * Rationale: the image must not be able to cause libelf to do
+ * unbounded work (ie, get stuck in a loop).
+ *
+ * - The input image and output area must be accessed only via the
+ * safe pointer access system. Real pointers into the input or
+ * output may not be even *calculated*.
+ *
+ * Rationale: calculating wild pointers is undefined behaviour;
+ * if the compiler sees that you might be calculating wild
+ * pointers, it may remove important checks!
+ *
+ * - Stack local buffer variables containing information derived from
+ * the image (including structs, or byte buffers) must be
+ * completely zeroed, using elf_memset_unchecked (and an
+ * accompanying elf_iter_ok_counted) on entry to the function (or
+ * somewhere very obviously near there).
+ *
+ * Rationale: This avoids uninitialised stack data being used
+ * as input to any of the loader.
+ *
+ * - All integer variables should be unsigned.
+ *
+ * Rationale: this avoids signed integer overflow (which has
+ * undefined behaviour in C, and if spotted by the compiler can
+ * cause it to generate bad code).
+ *
+ * - Arithmetic operations other than + - * should be avoided; in
+ * particular, division (/ or %) by non-constant values should be
+ * avoided. If it cannot be avoided, the divisor must be checked
+ * for zero.
+ *
+ * Rationale: We must avoid division-by-zero (or other overflow
+ * traps).
+ *
+ * - If it is desirable to breach these rules, there should be a
+ * comment explaining why this is OK.
+ *
+ * Even so, this is a fairly high-risk environment, so:
+ *
+ * - Do not add code which is not necessary for libelf to function
+ * with correct input, or to avoid being vulnerable to incorrect
+ * input. Do not add additional functionally-unnecessary checks
+ * for diagnosing problems with the image, or validating sanity of
+ * the input ELF.
+ *
+ * Rationale: Redundant checks have almost zero benefit because
+ * 1. we do not expect ELF-generating tools to generate invalid
+ * ELFs so these checks' failure paths will very likely never
+ * be executed anywhere, and 2. anyone debugging such a
+ * hypothetical bad ELF will have a variety of tools available
+ * which will do a much better job of analysing it.
+ *
+ * - However, it is OK to have checks code which provide duplicate
+ * defence against certain hostile images, if it is not otherwise
+ * obvious how libelf would be defended against such images.
+ *
+ * Rationale: Redundant checks where the situation would
+ * otherwise not be quite clear mean that the safety of the
+ * code is easy to see throughout; so that any unsafe code
+ * would be more obvious.
+ */
+
/* Macros for accessing the input image and output area. */
/*
@@ -475,6 +565,8 @@ static inline void *elf_memset_unchecked(void *s, int c,
size_t n)
* pointers. These are just like the real functions.
* We provide these so that in libelf-private.h we can #define
* memcpy, memset and memmove to undefined MISTAKE things.
+ *
+ * Remember to call elf_iter_ok_counted nearby.
*/
static inline int elf_strcmp_safe(struct elf_binary *elf,
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
