[groff] 13/13: [libbib]: Validate input to avoid heap overread.

[G. Branden Robinson]

gbranden pushed a commit to branch master
in repository groff.

commit 1b97881fc0e2246cf29b4758139a665c7816ba23
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
AuthorDate: Sun Sep 12 05:53:43 2021 +1000

    [libbib]: Validate input to avoid heap overread.
    
    Since June 1991 if not earlier, groff (technically, the refer, lookbib,
    and lkbib programs) has trusted the header contents of binary
    bibliographic index files (canonically generated with indxbib(1))
    regarding the sizes of the data structures that follow in the file, a
    notorious class of security problem.  In July 2013, the Mayhem Team at
    Carnegie Mellon University reported to the Debian Bug Tracking System a
    problem with groff's refer(1) implementation dumping core when reading
    an index file prepared by a fuzzer.
    
    * src/libs/libbib/index.cpp (index_search_item::check_header): Add new
      member function to validate the header of an indexed bibliography
      file, returning a string describing in detail the first validity
      problem encountered.
    
      (index_search_item::load): Perform the foregoing check before using
      any of the size values taken from the header; they are relied upon for
      pointer arithmetic.  If in verification mode (using the undocumented
      `-V` flag to refer(1), lkbib(1), or lookbib(1)), report the details of
      the problem encountered.  Regardless of that mode, if there is a
      validity problem, report corruption of the index file and abandon it,
      forcing fallback to the text version of the corresponding bibliography
      file.
    
    Fixes <https://bugs.debian.org/716109>.
---
 ChangeLog                 | 27 +++++++++++++++++++++++++++
 src/libs/libbib/index.cpp | 43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+)

diff --git a/ChangeLog b/ChangeLog
index fa7a83f..96c5419 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,32 @@
 2021-09-12  G. Branden Robinson <g.branden.robinson@gmail.com>
 
+       Since June 1991 if not earlier, groff (technically, the refer,
+       lookbib, and lkbib programs) has trusted the header contents of
+       binary bibliographic index files (canonically generated with
+       indxbib(1)) regarding the sizes of the data structures that
+       follow in the file, a notorious class of security problem.  In
+       July 2013, the Mayhem Team at Carnegie Mellon University
+       reported to the Debian Bug Tracking System a problem with
+       groff's refer(1) implementation dumping core when reading an
+       index file prepared by a fuzzer.
+
+       * src/libs/libbib/index.cpp (index_search_item::check_header):
+       Add new member function to validate the header of an indexed
+       bibliography file, returning a string describing in detail the
+       first validity problem encountered.
+       (index_search_item::load): Perform the foregoing check before
+       using any of the size values taken from the header; they are
+       relied upon for pointer arithmetic.  If in verification mode
+       {using the undocumented `-V` flag to refer(1), lkbib(1), or
+       lookbib(1)}, report the details of the problem encountered.
+       Regardless of that mode, if there is a validity problem, report
+       corruption of the index file and abandon it, forcing fallback to
+       the text version of the corresponding bibliography file.
+
+       Fixes <https://bugs.debian.org/716109>.
+
+2021-09-12  G. Branden Robinson <g.branden.robinson@gmail.com>
+
        * src/libs/libbib/index.cpp
        (index_search_item::get_invalidity_reason): Don't complain about
        about a last list element being nonnegative if the list size was
diff --git a/src/libs/libbib/index.cpp b/src/libs/libbib/index.cpp
index c984a71..3d14188 100644
--- a/src/libs/libbib/index.cpp
+++ b/src/libs/libbib/index.cpp
@@ -76,6 +76,7 @@ class index_search_item : public search_item {
 public:
   index_search_item(const char *, int);
   ~index_search_item();
+  const char *check_header(int);
   bool load(int fd);
   search_item_iterator *make_search_item_iterator(const char *);
   bool is_valid();
@@ -142,6 +143,40 @@ public:
 // Tell the compiler that a variable is intentionally unused.
 inline void unused(void *) { }
 
+// Validate the data reported in the header so that we don't overread on
+// the heap in the load() member function.  Return null pointer if no
+// problems are detected.
+const char *index_search_item::check_header(int size_remaining)
+{
+  size_t chunk_size;
+  if (header.tags_size < 0)
+    return "tag list length supposedly negative";
+  chunk_size = header.tags_size * sizeof(tag);
+  if (chunk_size > size_remaining)
+    return "claimed tag list length exceeds file size";
+  size_remaining -= chunk_size;
+  if (header.lists_size < 0)
+    return "reference list length supposedly negative";
+  chunk_size = header.lists_size * sizeof(int);
+  if (chunk_size > size_remaining)
+    return "claimed reference list length exceeds file size";
+  size_remaining -= chunk_size;
+  // The table and string pool sizes will not be zero, even in an empty
+  // index.
+  if (header.table_size < 1)
+    return "table size supposedly nonpositive";
+  chunk_size = header.table_size * sizeof(int);
+  if (chunk_size > size_remaining)
+    return "claimed table size exceeds file size";
+  size_remaining -= chunk_size;
+  if (header.strings_size < 1)
+    return "string pool size supposedly nonpositive";
+  chunk_size = header.strings_size;
+  if (chunk_size > size_remaining)
+    return "claimed string pool size exceeds file size";
+  return 0;
+}
+
 bool index_search_item::load(int fd)
 {
   file_closer fd_closer(fd);   // close fd on return
@@ -209,6 +244,14 @@ bool index_search_item::load(int fd)
          name, size, sz);
     return false;
   }
+  const char *problem = check_header(size);
+  if (problem) {
+    if (do_verify)
+      error("corrupt header in index file '%1': %2", name, problem);
+    else
+      error("corrupt header in index file '%1'", name);
+    return false;
+  }
   tags = (tag *)(addr + sizeof(header));
   lists = (int *)(tags + header.tags_size);
   table = (int *)(lists + header.lists_size);