[Xen-devel] [PATCH 0/8] libelf: safety enhancements

[Ian Jackson <ian.jackson@xxxxxxxxxxxxx>]

We recently discovered two near-miss in libelf:

* The intended method for limiting the phdr loop iteration count was
  not effective.  But happily this turned not to be important because
  the count field is only 16 bits.

* A recent commit accidentally introduced a division by zero
  vulnerability.

Subsequent discussion revealed that the design principles underlying
libelf's safety were not widely understood - because they were not
documented.

Initially I tried dealing with the loop safety problem by auditing the
code and adding a suitable comment next to each loop, stating a proof
sketch of the loop's safety.  I found that this quickly became
unworkable, because there are nested loops.  These nested loops did
not have badly unreasonable upper bounds but the complexity of the
analysis was unsuitable for security-critical review.

An upper bound on the work done in loops in libelf is necessary
because libelf may be called by the toolstack in a context where it
would block other work.  Specifically, libelf is called by libxl, and
libxl does all of its work within a single per-ctx lock.  libxl's
callers are not supposed to be required to invoke libxl on multiple
ctxs or with multiple processes simultaneously, and in any case
we don't want to generate and leak stuck toolstack processes.

So, in this series, I propose:

 * A new scheme for limiting the work done by libelf.  We track it
   explicitly, and check it on each iteration of every loop.  (This
   replaces a similar ad-hoc scheme used for copying image data.)

 * Documentation which states the safety design principles for libelf,
   and the coding rules which follow from those design principles.

After this series is done there are a few redundant loop safety
checks, from the previous approach:

 * There are a number of ad-hoc limits on string sizes, certain table
   sizes, etc.

 * There are calls to elf_access_ok which were intended to limit loop
   iteration counts (but are ineffective at doing so since the stride
   is controlled by the input image and might be zero).

I have chosen to retain these.  Removing them seems like an
unnecessary risk.  In particular, searching for and removing
certain elf_access_ok calls seems unwise.

Thanks,
Ian.

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