[cairo-commit] 2 commits - src/cairo-array.c src/cairo-truetype-subset.c

[GitLab Mirror]

 src/cairo-array.c           |    5 +++++
 src/cairo-truetype-subset.c |   22 ++++++++++------------
 2 files changed, 15 insertions(+), 12 deletions(-)

New commits:
commit 1569dcd4d8a2235052aa227dc9f5708cccaea23e
Merge: c62c43a8c 967ac9378
Author: Heiko Lewin <hlewin at gmx.de>
Date:   Sun Feb 21 16:01:41 2021 +0000

    Merge branch 'ubsan-fixes' into 'master'
    
    UBSan fixes in cairo-truetype-subset.c
    
    See merge request cairo/cairo!124

commit 967ac93789b1ce4e3f950f8eb6bc563ad91abeab
Author: Jonathan Kew <jfkthame at googlemail.com>
Date:   Sun Feb 21 16:01:40 2021 +0000

    Don't call _cairo_array_append_multiple with a zero count.
    
    The documentation for _cairo_array_append_multiple says "one or more items".
    If it is called with num_elements=0, it ends up calling _cairo_array_grow_by
    with num_elements=0, which if the array is currently empty (as here) leads
    to undefined behavior in _cairo_array_allocate in the line
    
        *elements = array->elements + array->num_elements * array->element_size;
    
    because it ends up trying to add 0 to a null pointer. C doesn't allow this.
    (UBSan flags this as "applying zero offset to null pointer".)

diff --git a/src/cairo-array.c b/src/cairo-array.c
index c43ed2e12..af8b7e982 100644
--- a/src/cairo-array.c
+++ b/src/cairo-array.c
@@ -505,6 +505,11 @@ _cairo_user_data_array_copy (cairo_user_data_array_t	*dst,
 	_cairo_user_data_array_init (dst);
     }
 
+    /* don't call _cairo_array_append_multiple if there's nothing to do,
+     * as it assumes at least 1 element is to be appended */
+    if (src->num_elements == 0)
+        return CAIRO_STATUS_SUCCESS;
+
     return _cairo_array_append_multiple (dst,
 					 _cairo_array_index_const (src, 0),
 					 src->num_elements);
diff --git a/src/cairo-truetype-subset.c b/src/cairo-truetype-subset.c
index 6cef4ee42..7f0445df4 100644
--- a/src/cairo-truetype-subset.c
+++ b/src/cairo-truetype-subset.c
@@ -1272,7 +1272,7 @@ _cairo_truetype_reverse_cmap (cairo_scaled_font_t *scaled_font,
     cairo_status_t status;
     const cairo_scaled_font_backend_t *backend;
     tt_segment_map_t *map;
-    char buf[4];
+    tt_segment_map_t map_header;
     unsigned int num_segments, i;
     unsigned long size;
     uint16_t *start_code;
@@ -1282,20 +1282,19 @@ _cairo_truetype_reverse_cmap (cairo_scaled_font_t *scaled_font,
     uint16_t  c;
 
     backend = scaled_font->backend;
-    size = 4;
+    size = 4;  /* enough to read the two header fields we need */
     status = backend->load_truetype_table (scaled_font,
                                            TT_TAG_cmap, table_offset,
-					   (unsigned char *) &buf,
+					   (unsigned char *) &map_header,
 					   &size);
     if (unlikely (status))
 	return status;
 
     /* All table formats have the same first two words */
-    map = (tt_segment_map_t *) buf;
-    if (be16_to_cpu (map->format) != 4)
+    if (be16_to_cpu (map_header.format) != 4)
 	return CAIRO_INT_STATUS_UNSUPPORTED;
 
-    size = be16_to_cpu (map->length);
+    size = be16_to_cpu (map_header.length);
     map = _cairo_malloc (size);
     if (unlikely (map == NULL))
 	return _cairo_error (CAIRO_STATUS_NO_MEMORY);
@@ -1382,7 +1381,7 @@ _cairo_truetype_index_to_ucs4 (cairo_scaled_font_t *scaled_font,
     cairo_int_status_t status = CAIRO_INT_STATUS_UNSUPPORTED;
     const cairo_scaled_font_backend_t *backend;
     tt_cmap_t *cmap;
-    char buf[4];
+    tt_cmap_t cmap_header;
     int num_tables, i;
     unsigned long size;
 
@@ -1390,17 +1389,16 @@ _cairo_truetype_index_to_ucs4 (cairo_scaled_font_t *scaled_font,
     if (!backend->load_truetype_table)
 	return CAIRO_INT_STATUS_UNSUPPORTED;
 
-    size = 4;
+    size = 4;  /* only read the header fields 'version' and 'num_tables' */
     status = backend->load_truetype_table (scaled_font,
                                            TT_TAG_cmap, 0,
-					   (unsigned char *) &buf,
+					   (unsigned char *) &cmap_header,
 					   &size);
     if (unlikely (status))
 	return status;
 
-    cmap = (tt_cmap_t *) buf;
-    num_tables = be16_to_cpu (cmap->num_tables);
-    size = 4 + num_tables*sizeof(tt_cmap_index_t);
+    num_tables = be16_to_cpu (cmap_header.num_tables);
+    size = 4 + num_tables * sizeof (tt_cmap_index_t);
     cmap = _cairo_malloc_ab_plus_c (num_tables, sizeof (tt_cmap_index_t), 4);
     if (unlikely (cmap == NULL))
 	return _cairo_error (CAIRO_STATUS_NO_MEMORY);