Fix out-of-bounds access in cff subset

I was looking at [1]. While trying to reproduce the problem that is
described there, valgrind reported:

 Argument 'size' of function malloc has a fishy (possibly negative) value: -8
    at 0x48407B4: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
    by 0x4B20E92: cairo_cff_font_read_name (cairo-cff-subset.c:895)
    by 0x4B221AD: cairo_cff_font_read_font (cairo-cff-subset.c:1351)
    by 0x4B24EF2: cairo_cff_font_generate (cairo-cff-subset.c:2587)
    by 0x4B25EA3: _cairo_cff_subset_init (cairo-cff-subset.c:2979)

This commit is about fixing the above.

The function decode_index_offset() returns an unsigned long. This value
was cast to an "int" in cff_index_read(), leading to a possibility for
over/underflow. Also, nothing checked that an entry in the index table
had a non-zero length, leading to an entry with length -8 as reported by
valgrind.

Fix this by using "unsigned long" for the local variables and checking
the length to be non-negative.

With the above fixed, the original test case started crashing.
Apparently, cairo_cff_font_read_name() does not expect nor handle
failures from cff_index_read(). Thus, a check for this case was added to
make the new crash go away.

[1]: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=51324

Signed-off-by: Uli Schlachter <psychon@znc.in>

1 files changed, 4 insertions, 4 deletions

diff --git a/src/cairo-cff-subset.c b/src/cairo-cff-subset.c
index 4bf22e2b7..38b6824b6 100644
--- a/src/cairo-cff-subset.c
+++ b/src/cairo-cff-subset.c
@@ -412,8 +412,8 @@ cff_index_read (cairo_array_t *index, unsigned char **ptr, unsigned char *end_pt
     cff_index_element_t element;
     unsigned char *data, *p;
     cairo_status_t status;
-    int offset_size, count, start, i;
-    int end = 0;
+    int offset_size, count, i;
+    unsigned long start, end = 0;
 
     p = *ptr;
     if (p + 2 > end_ptr)
@@ -430,7 +430,7 @@ cff_index_read (cairo_array_t *index, unsigned char **ptr, unsigned char *end_pt
         for (i = 0; i < count; i++) {
             end = decode_index_offset (p, offset_size);
             p += offset_size;
-            if (p > end_ptr)
+            if (p > end_ptr || end < start)
                 return CAIRO_INT_STATUS_UNSUPPORTED;
             element.length = end - start;
             element.is_copy = FALSE;
@@ -875,7 +875,7 @@ cairo_cff_font_read_name (cairo_cff_font_t *font)
 
     cff_index_init (&index);
     status = cff_index_read (&index, &font->current_ptr, font->data_end);
-    if (!font->is_opentype) {
+    if (status == CAIRO_INT_STATUS_SUCCESS && !font->is_opentype) {
         element = _cairo_array_index (&index, 0);
 	p = element->data;
 	len = element->length;