[cairo-commit] 4 commits - src/cairo-array.c src/cairoint.h src/cairo-truetype-subset.c

Chris Wilson ickle at kemper.freedesktop.org
Thu Apr 3 10:52:26 PDT 2008


 src/cairo-array.c           |   12 +++++---
 src/cairo-truetype-subset.c |   63 ++++++++++++++++++++++++++++++++------------
 src/cairoint.h              |    2 -
 3 files changed, 56 insertions(+), 21 deletions(-)

New commits:
commit bb76eb508b2d97a4455393a12540ceb7427bc271
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Thu Apr 3 18:36:16 2008 +0100

    [cairo-truetype-subset] Check reads are within valid data.
    
    Check cairo_truetype_font_remap_composite_glyph() does not read beyond
    the end of the buffer loaded for the glyf.

diff --git a/src/cairo-truetype-subset.c b/src/cairo-truetype-subset.c
index db3f95b..3fa174c 100644
--- a/src/cairo-truetype-subset.c
+++ b/src/cairo-truetype-subset.c
@@ -494,11 +494,12 @@ cairo_truetype_font_write_generic_table (cairo_truetype_font_t *font,
 }
 
 static cairo_status_t
-cairo_truetype_font_remap_composite_glyph (cairo_truetype_font_t *font,
-					   unsigned char         *buffer)
+cairo_truetype_font_remap_composite_glyph (cairo_truetype_font_t	*font,
+					   unsigned char		*buffer,
+					   unsigned long		 size)
 {
     tt_glyph_data_t *glyph_data;
-    tt_composite_glyph_t *composite_glyph;
+    tt_composite_glyph_t *composite_glyph, *last_glyph;
     int num_args;
     int has_more_components;
     unsigned short flags;
@@ -508,11 +509,15 @@ cairo_truetype_font_remap_composite_glyph (cairo_truetype_font_t *font,
     if (font->status)
 	return font->status;
 
+    if (size < sizeof (tt_glyph_data_t))
+	return CAIRO_INT_STATUS_UNSUPPORTED;
+
     glyph_data = (tt_glyph_data_t *) buffer;
     if ((int16_t)be16_to_cpu (glyph_data->num_contours) >= 0)
         return CAIRO_STATUS_SUCCESS;
 
     composite_glyph = &glyph_data->glyph;
+    last_glyph = (tt_composite_glyph_t *) (buffer + size);
     do {
         flags = be16_to_cpu (composite_glyph->flags);
         has_more_components = flags & TT_MORE_COMPONENTS;
@@ -531,6 +536,9 @@ cairo_truetype_font_remap_composite_glyph (cairo_truetype_font_t *font,
         else if (flags & TT_WE_HAVE_A_TWO_BY_TWO)
             num_args += 3;
         composite_glyph = (tt_composite_glyph_t *) &(composite_glyph->args[num_args]);
+
+	if (has_more_components && composite_glyph >= last_glyph)
+	    return CAIRO_INT_STATUS_UNSUPPORTED;
     } while (has_more_components);
 
     return CAIRO_STATUS_SUCCESS;
@@ -615,7 +623,7 @@ cairo_truetype_font_write_glyf_table (cairo_truetype_font_t *font,
 	    if (status)
 		goto FAIL;
 
-            status = cairo_truetype_font_remap_composite_glyph (font, buffer);
+            status = cairo_truetype_font_remap_composite_glyph (font, buffer, size);
 	    if (status)
 		goto FAIL;
         }
commit a5e2a2ad2d03c217b9b27c537ee6a945bdc98a44
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Thu Apr 3 18:24:00 2008 +0100

    [cairo-truetype-subset] Prevent accesses beyond the end of the glyph array.
    
    Reject the font if we try to remap a composite glyph that exists outside
    the known set of glyphs.

diff --git a/src/cairo-truetype-subset.c b/src/cairo-truetype-subset.c
index 68c5ff3..db3f95b 100644
--- a/src/cairo-truetype-subset.c
+++ b/src/cairo-truetype-subset.c
@@ -85,8 +85,10 @@ struct _cairo_truetype_font {
 
 };
 
-static int
-cairo_truetype_font_use_glyph (cairo_truetype_font_t *font, int glyph);
+static cairo_status_t
+cairo_truetype_font_use_glyph (cairo_truetype_font_t	    *font,
+	                       unsigned short		     glyph,
+			       unsigned short		    *out);
 
 #define SFNT_VERSION			0x00010000
 #define SFNT_STRING_MAX_LENGTH  65535
@@ -491,7 +493,7 @@ cairo_truetype_font_write_generic_table (cairo_truetype_font_t *font,
     return CAIRO_STATUS_SUCCESS;
 }
 
-static void
+static cairo_status_t
 cairo_truetype_font_remap_composite_glyph (cairo_truetype_font_t *font,
 					   unsigned char         *buffer)
 {
@@ -501,19 +503,23 @@ cairo_truetype_font_remap_composite_glyph (cairo_truetype_font_t *font,
     int has_more_components;
     unsigned short flags;
     unsigned short index;
+    cairo_status_t status;
 
     if (font->status)
-	return;
+	return font->status;
 
     glyph_data = (tt_glyph_data_t *) buffer;
     if ((int16_t)be16_to_cpu (glyph_data->num_contours) >= 0)
-        return;
+        return CAIRO_STATUS_SUCCESS;
 
     composite_glyph = &glyph_data->glyph;
     do {
         flags = be16_to_cpu (composite_glyph->flags);
         has_more_components = flags & TT_MORE_COMPONENTS;
-        index = cairo_truetype_font_use_glyph (font, be16_to_cpu (composite_glyph->index));
+        status = cairo_truetype_font_use_glyph (font, be16_to_cpu (composite_glyph->index), &index);
+	if (status)
+	    return status;
+
         composite_glyph->index = cpu_to_be16 (index);
         num_args = 1;
         if (flags & TT_ARG_1_AND_2_ARE_WORDS)
@@ -526,6 +532,8 @@ cairo_truetype_font_remap_composite_glyph (cairo_truetype_font_t *font,
             num_args += 3;
         composite_glyph = (tt_composite_glyph_t *) &(composite_glyph->args[num_args]);
     } while (has_more_components);
+
+    return CAIRO_STATUS_SUCCESS;
 }
 
 static cairo_status_t
@@ -607,7 +615,9 @@ cairo_truetype_font_write_glyf_table (cairo_truetype_font_t *font,
 	    if (status)
 		goto FAIL;
 
-            cairo_truetype_font_remap_composite_glyph (font, buffer);
+            status = cairo_truetype_font_remap_composite_glyph (font, buffer);
+	    if (status)
+		goto FAIL;
         }
     }
 
@@ -938,16 +948,22 @@ cairo_truetype_font_generate (cairo_truetype_font_t  *font,
     return _cairo_truetype_font_set_error (font, status);
 }
 
-static int
-cairo_truetype_font_use_glyph (cairo_truetype_font_t *font, int glyph)
+static cairo_status_t
+cairo_truetype_font_use_glyph (cairo_truetype_font_t	    *font,
+	                       unsigned short		     glyph,
+			       unsigned short		    *out)
 {
+    if (glyph >= font->num_glyphs_in_face)
+	return CAIRO_INT_STATUS_UNSUPPORTED;
+
     if (font->parent_to_subset[glyph] == 0) {
 	font->parent_to_subset[glyph] = font->base.num_glyphs;
 	font->glyphs[font->base.num_glyphs].parent_index = glyph;
 	font->base.num_glyphs++;
     }
 
-    return font->parent_to_subset[glyph];
+    *out = font->parent_to_subset[glyph];
+    return CAIRO_STATUS_SUCCESS;
 }
 
 static void
@@ -1043,7 +1059,7 @@ _cairo_truetype_subset_init (cairo_truetype_subset_t    *truetype_subset,
     cairo_status_t status;
     const char *data = NULL; /* squelch bogus compiler warning */
     unsigned long length = 0; /* squelch bogus compiler warning */
-    unsigned long parent_glyph, offsets_length;
+    unsigned long offsets_length;
     unsigned int i;
     const unsigned long *string_offsets = NULL;
     unsigned long num_strings = 0;
@@ -1053,8 +1069,9 @@ _cairo_truetype_subset_init (cairo_truetype_subset_t    *truetype_subset,
 	return status;
 
     for (i = 0; i < font->scaled_font_subset->num_glyphs; i++) {
-	parent_glyph = font->scaled_font_subset->glyphs[i];
-	cairo_truetype_font_use_glyph (font, parent_glyph);
+	unsigned short parent_glyph = font->scaled_font_subset->glyphs[i];
+	status = cairo_truetype_font_use_glyph (font, parent_glyph, &parent_glyph);
+	assert (status == CAIRO_STATUS_SUCCESS);
     }
 
     cairo_truetype_font_create_truetype_table_list (font);
commit 13cdfed894d48b30e28296c3a27c8361bf5506fb
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Thu Apr 3 17:43:37 2008 +0100

    [cairo-truetype-subset] Perform a quick sanity check that glyf end >= begin.
    
    Check for a bogus glyf position and prevent an integer overflow.

diff --git a/src/cairo-truetype-subset.c b/src/cairo-truetype-subset.c
index f143882..68c5ff3 100644
--- a/src/cairo-truetype-subset.c
+++ b/src/cairo-truetype-subset.c
@@ -580,6 +580,12 @@ cairo_truetype_font_write_glyf_table (cairo_truetype_font_t *font,
 	    end = be32_to_cpu (u.long_offsets[index + 1]);
 	}
 
+	/* quick sanity check... */
+	if (end < begin) {
+	    status = CAIRO_INT_STATUS_UNSUPPORTED;
+	    goto FAIL;
+	}
+
 	size = end - begin;
         status = cairo_truetype_font_align_output (font, &next);
 	if (status)
commit cfff3c3bd04df5257176d9e43add52fc6daba329
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Thu Apr 3 17:23:48 2008 +0100

    [cairo-array] Guard against integer overflow whilst growing the array.
    
    Sanity check the arguments to _cairo_array_grow_by() such that the
    array size does not overflow, similar to the defensive checking of
    parameters to malloc.

diff --git a/src/cairo-array.c b/src/cairo-array.c
index b547b12..053e73e 100644
--- a/src/cairo-array.c
+++ b/src/cairo-array.c
@@ -110,15 +110,19 @@ _cairo_array_fini (cairo_array_t *array)
  * is always increased by doubling as many times as necessary.
  **/
 cairo_status_t
-_cairo_array_grow_by (cairo_array_t *array, int additional)
+_cairo_array_grow_by (cairo_array_t *array, unsigned int additional)
 {
     char *new_elements;
-    int old_size = array->size;
-    int required_size = array->num_elements + additional;
-    int new_size;
+    unsigned int old_size = array->size;
+    unsigned int required_size = array->num_elements + additional;
+    unsigned int new_size;
 
     assert (! array->is_snapshot);
 
+    /* check for integer overflow */
+    if (required_size > INT_MAX || required_size < array->num_elements)
+	return _cairo_error (CAIRO_STATUS_NO_MEMORY);
+
     if (required_size <= old_size)
 	return CAIRO_STATUS_SUCCESS;
 
diff --git a/src/cairoint.h b/src/cairoint.h
index 6a89d71..01ad567 100644
--- a/src/cairoint.h
+++ b/src/cairoint.h
@@ -231,7 +231,7 @@ cairo_private void
 _cairo_array_fini (cairo_array_t *array);
 
 cairo_private cairo_status_t
-_cairo_array_grow_by (cairo_array_t *array, int additional);
+_cairo_array_grow_by (cairo_array_t *array, unsigned int additional);
 
 cairo_private void
 _cairo_array_truncate (cairo_array_t *array, unsigned int num_elements);


More information about the cairo-commit mailing list