From 5858fa16cace81fb7b3de7600cb765a820348bed Mon Sep 17 00:00:00 2001 From: Ben Wagner Date: Tue, 7 May 2024 18:19:58 -0400 Subject: [PATCH] [colr] Avoid overflow in range checks In 32 bit builds `FT_ULong` is 32 bits and can silently overflow when a large number is read into one and then it is summed or multiplied with another number. Checks for range overflow must be written so that they themselves do not overflow. Also ensure that the table_size is always the first part of the range check and consistently use `<` or `<=`. * src/sfnt/ttcolr.c (tt_face_load_colr): Avoid overflow. (find_base_glyph_v1_record): Remove old work-around. Bug: https://issues.chromium.org/issues/41495455 Bug: https://issues.chromium.org/issues/40945818 --- src/sfnt/ttcolr.c | 39 +++++++++++++++------------------------ 1 file changed, 15 insertions(+), 24 deletions(-) diff --git a/src/sfnt/ttcolr.c b/src/sfnt/ttcolr.c index 1c3fd70d0..b37658dde 100644 --- a/src/sfnt/ttcolr.c +++ b/src/sfnt/ttcolr.c @@ -208,18 +208,19 @@ colr->num_base_glyphs = FT_NEXT_USHORT( p ); base_glyph_offset = FT_NEXT_ULONG( p ); - if ( base_glyph_offset >= table_size ) + if ( table_size <= base_glyph_offset ) goto InvalidTable; - if ( colr->num_base_glyphs * BASE_GLYPH_SIZE > - table_size - base_glyph_offset ) + if ( ( table_size - base_glyph_offset ) / BASE_GLYPH_SIZE + < colr->num_base_glyphs ) goto InvalidTable; layer_offset = FT_NEXT_ULONG( p ); colr->num_layers = FT_NEXT_USHORT( p ); - if ( layer_offset >= table_size ) + if ( table_size <= layer_offset ) goto InvalidTable; - if ( colr->num_layers * LAYER_SIZE > table_size - layer_offset ) + if ( ( table_size - layer_offset ) / LAYER_SIZE + < colr->num_layers ) goto InvalidTable; if ( colr->version == 1 ) @@ -229,14 +230,14 @@ base_glyphs_offset_v1 = FT_NEXT_ULONG( p ); - if ( base_glyphs_offset_v1 >= table_size - 4 ) + if ( table_size - 4 <= base_glyphs_offset_v1 ) goto InvalidTable; p1 = (FT_Byte*)( table + base_glyphs_offset_v1 ); num_base_glyphs_v1 = FT_PEEK_ULONG( p1 ); - if ( num_base_glyphs_v1 * BASE_GLYPH_PAINT_RECORD_SIZE > - table_size - base_glyphs_offset_v1 ) + if ( ( table_size - base_glyphs_offset_v1 ) / BASE_GLYPH_PAINT_RECORD_SIZE + < num_base_glyphs_v1 ) goto InvalidTable; colr->num_base_glyphs_v1 = num_base_glyphs_v1; @@ -244,19 +245,19 @@ layer_offset_v1 = FT_NEXT_ULONG( p ); - if ( layer_offset_v1 >= table_size ) + if ( table_size <= layer_offset_v1 ) goto InvalidTable; if ( layer_offset_v1 ) { - if ( layer_offset_v1 >= table_size - 4 ) + if ( table_size - 4 <= layer_offset_v1 ) goto InvalidTable; p1 = (FT_Byte*)( table + layer_offset_v1 ); num_layers_v1 = FT_PEEK_ULONG( p1 ); - if ( num_layers_v1 * LAYER_V1_LIST_PAINT_OFFSET_SIZE > - table_size - layer_offset_v1 ) + if ( ( table_size - layer_offset_v1 ) / LAYER_V1_LIST_PAINT_OFFSET_SIZE + < num_layers_v1 ) goto InvalidTable; colr->num_layers_v1 = num_layers_v1; @@ -279,7 +280,7 @@ clip_list_offset = FT_NEXT_ULONG( p ); - if ( clip_list_offset >= table_size ) + if ( table_size <= clip_list_offset ) goto InvalidTable; if ( clip_list_offset ) @@ -311,7 +312,7 @@ goto InvalidTable; var_store_offset = FT_NEXT_ULONG( p ); - if ( var_store_offset >= table_size ) + if ( table_size <= var_store_offset ) goto InvalidTable; if ( var_store_offset ) @@ -1270,7 +1271,6 @@ static FT_Bool find_base_glyph_v1_record( FT_Byte * base_glyph_begin, FT_UInt num_base_glyph, - FT_Byte * end_colr, FT_UInt glyph_id, BaseGlyphV1Record *record ) { @@ -1290,14 +1290,6 @@ */ FT_Byte *p = base_glyph_begin + 4 + mid * BASE_GLYPH_PAINT_RECORD_SIZE; - - /* We need to be able to read 2 bytes (FT_NEXT_USHORT) for the glyph */ - /* ID, then 4 bytes (FT_NEXT_ULONG) for the paint offset. If that's */ - /* not available before the end of the table, something's wrong with */ - /* the font and we can't find a COLRv1 glyph. */ - if ( p > end_colr - 2 - 4 ) - return 0; - gid = FT_NEXT_USHORT( p ); if ( gid < glyph_id ) @@ -1338,7 +1330,6 @@ if ( !find_base_glyph_v1_record( colr->base_glyphs_v1, colr->num_base_glyphs_v1, - (FT_Byte*)colr->table + colr->table_size, base_glyph, &base_glyph_v1_record ) ) return 0;