From ded4bdb5d0fce69fe0ac804b15c695a71b31d3ad Mon Sep 17 00:00:00 2001 From: Werner Lemberg Date: Tue, 20 Dec 2016 23:26:38 +0100 Subject: [PATCH] [base] More sanity checks for Mac resources. We use https://github.com/kreativekorp/ksfl/wiki/Macintosh-Resource-File-Format and https://developer.apple.com/legacy/library/documentation/mac/pdf/MoreMacintoshToolbox.pdf#page=151 as references. * include/freetype/internal/ftrfork.h (FT_RFork_Ref): Use FT_Short for `res_id'. * src/base/ftrfork.c (FT_Raccess_Get_HeaderInfo): Extract map length and use it to improve sanity checks. Follow the specification more closely;in particular, all data types are signed, not unsigned. (FT_Raccess_Get_DataOffsets): Follow the specification more closely; in particular, all data types are signed, not unsigned. Add some sanity checks. --- ChangeLog | 25 +++++++ include/freetype/internal/ftrfork.h | 5 +- src/base/ftrfork.c | 107 +++++++++++++++++++++------- 3 files changed, 109 insertions(+), 28 deletions(-) diff --git a/ChangeLog b/ChangeLog index 371968394..8ecd24224 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,28 @@ +2016-12-20 Werner Lemberg + + [base] More sanity checks for Mac resources. + + We use + + https://github.com/kreativekorp/ksfl/wiki/Macintosh-Resource-File-Format + + and + + https://developer.apple.com/legacy/library/documentation/mac/pdf/MoreMacintoshToolbox.pdf#page=151 + + as references. + + * include/freetype/internal/ftrfork.h (FT_RFork_Ref): Use FT_Short + for `res_id'. + + * src/base/ftrfork.c (FT_Raccess_Get_HeaderInfo): Extract map length + and use it to improve sanity checks. + Follow the specification more closely;in particular, all data types + are signed, not unsigned. + (FT_Raccess_Get_DataOffsets): Follow the specification more closely; + in particular, all data types are signed, not unsigned. + Add some sanity checks. + 2016-12-20 Werner Lemberg [truetype] Improve logic for getting fast advance widths. diff --git a/include/freetype/internal/ftrfork.h b/include/freetype/internal/ftrfork.h index b923401e6..718fa6268 100644 --- a/include/freetype/internal/ftrfork.h +++ b/include/freetype/internal/ftrfork.h @@ -43,11 +43,12 @@ FT_BEGIN_HEADER typedef struct FT_RFork_Ref_ { - FT_UShort res_id; - FT_Long offset; + FT_Short res_id; + FT_Long offset; } FT_RFork_Ref; + #ifdef FT_CONFIG_OPTION_GUESSING_EMBEDDED_RFORK typedef FT_Error (*ft_raccess_guess_func)( FT_Library library, diff --git a/src/base/ftrfork.c b/src/base/ftrfork.c index 4660c971c..b8b97a826 100644 --- a/src/base/ftrfork.c +++ b/src/base/ftrfork.c @@ -56,7 +56,7 @@ { FT_Error error; unsigned char head[16], head2[16]; - FT_Long map_pos, rdata_len; + FT_Long map_pos, map_len, rdata_len; int allzeros, allmatch, i; FT_Long type_list; @@ -67,12 +67,15 @@ if ( error ) return error; - error = FT_Stream_Read( stream, (FT_Byte *)head, 16 ); + error = FT_Stream_Read( stream, (FT_Byte*)head, 16 ); if ( error ) return error; /* ensure positive values */ - if ( head[0] >= 0x80 || head[4] >= 0x80 || head[8] >= 0x80 ) + if ( head[0] >= 0x80 || + head[4] >= 0x80 || + head[8] >= 0x80 || + head[12] >= 0x80 ) return FT_THROW( Unknown_File_Format ); *rdata_pos = ( head[ 0] << 24 ) | @@ -87,14 +90,36 @@ ( head[ 9] << 16 ) | ( head[10] << 8 ) | head[11]; + map_len = ( head[12] << 24 ) | + ( head[13] << 16 ) | + ( head[14] << 8 ) | + head[15]; - /* map_len = head[12] .. head[15] */ - - if ( *rdata_pos != map_pos - rdata_len || map_pos == 0 ) + /* the map must not be empty */ + if ( !map_pos ) return FT_THROW( Unknown_File_Format ); - if ( FT_LONG_MAX - rfork_offset < *rdata_pos || - FT_LONG_MAX - rfork_offset < map_pos ) + /* check whether rdata and map overlap */ + if ( *rdata_pos < map_pos ) + { + if ( *rdata_pos > map_pos - rdata_len ) + return FT_THROW( Unknown_File_Format ); + } + else + { + if ( map_pos > *rdata_pos - map_len ) + return FT_THROW( Unknown_File_Format ); + } + + /* check whether end of rdata or map exceeds stream size */ + if ( FT_LONG_MAX - rdata_len < *rdata_pos || + FT_LONG_MAX - map_len < map_pos || + + FT_LONG_MAX - ( *rdata_pos + rdata_len ) < rfork_offset || + FT_LONG_MAX - ( map_pos + map_len ) < rfork_offset || + + (FT_ULong)( rfork_offset + *rdata_pos + rdata_len ) > stream->size || + (FT_ULong)( rfork_offset + map_pos + map_len ) > stream->size ) return FT_THROW( Unknown_File_Format ); *rdata_pos += rfork_offset; @@ -124,15 +149,14 @@ /* If we have reached this point then it is probably a mac resource */ /* file. Now, does it contain any interesting resources? */ - /* Skip handle to next resource map, the file resource number, and */ - /* attributes. */ + (void)FT_STREAM_SKIP( 4 /* skip handle to next resource map */ + 2 /* skip file resource number */ + 2 ); /* skip attributes */ - if ( FT_READ_USHORT( type_list ) ) + if ( FT_READ_SHORT( type_list ) ) return error; - if ( type_list == -1 ) + if ( type_list < 0 ) return FT_THROW( Unknown_File_Format ); error = FT_Stream_Seek( stream, (FT_ULong)( map_pos + type_list ) ); @@ -181,15 +205,34 @@ if ( error ) return error; - if ( FT_READ_USHORT( cnt ) ) + if ( FT_READ_SHORT( cnt ) ) return error; cnt++; + /* `rpos' is a signed 16bit integer offset to resource records; the */ + /* size of a resource record is 12 bytes. The map header is 28 bytes, */ + /* and a type list needs 10 bytes or more. If we assume that the name */ + /* list is empty and we have only a single entry in the type list, */ + /* there can be at most */ + /* */ + /* (32768 - 28 - 10) / 12 = 2727 */ + /* */ + /* resources. */ + /* */ + /* A type list starts with a two-byte counter, followed by 10-byte */ + /* type records. Assuming that there are no resources, the number of */ + /* type records can be at most */ + /* */ + /* (32768 - 28 - 2) / 8 = 4079 */ + /* */ + if ( cnt > 4079 ) + return FT_THROW( Invalid_Table ); + for ( i = 0; i < cnt; ++i ) { if ( FT_READ_LONG( tag_internal ) || - FT_READ_USHORT( subcnt ) || - FT_READ_USHORT( rpos ) ) + FT_READ_SHORT( subcnt ) || + FT_READ_SHORT( rpos ) ) return error; FT_TRACE2(( "Resource tags: %c%c%c%c\n", @@ -205,6 +248,9 @@ *count = subcnt + 1; rpos += map_offset; + if ( *count > 2727 ) + return FT_THROW( Invalid_Table ); + error = FT_Stream_Seek( stream, (FT_ULong)rpos ); if ( error ) return error; @@ -214,33 +260,42 @@ for ( j = 0; j < *count; ++j ) { - if ( FT_READ_USHORT( ref[j].res_id ) ) + if ( FT_READ_SHORT( ref[j].res_id ) ) goto Exit; - if ( FT_STREAM_SKIP( 2 ) ) /* resource name */ + if ( FT_STREAM_SKIP( 2 ) ) /* resource name offset */ goto Exit; - if ( FT_READ_LONG( temp ) ) + if ( FT_READ_LONG( temp ) ) /* attributes (8bit), offset (24bit) */ goto Exit; - if ( FT_STREAM_SKIP( 4 ) ) /* mbz */ + if ( FT_STREAM_SKIP( 4 ) ) /* mbz */ goto Exit; + if ( ref[j].res_id < 0 || temp < 0 ) + { + error = FT_THROW( Invalid_Table ); + goto Exit; + } + ref[j].offset = temp & 0xFFFFFFL; + FT_TRACE3(( " [%d]:" " resource_id=0x%04x, offset=0x%08x\n", j, ref[j].res_id, ref[j].offset )); } - if (sort_by_res_id) + if ( sort_by_res_id ) { - ft_qsort( ref, (size_t)*count, sizeof ( FT_RFork_Ref ), - ( int(*)(const void*, const void*) ) - ft_raccess_sort_ref_by_id ); + ft_qsort( ref, + (size_t)*count, + sizeof ( FT_RFork_Ref ), + ( int(*)(const void*, + const void*) )ft_raccess_sort_ref_by_id ); FT_TRACE3(( " -- sort resources by their ids --\n" )); - for ( j = 0; j < *count; ++ j ) { + + for ( j = 0; j < *count; j++ ) FT_TRACE3(( " [%d]:" " resource_id=0x%04x, offset=0x%08x\n", j, ref[j].res_id, ref[j].offset )); - } } if ( FT_NEW_ARRAY( offsets_internal, *count ) ) @@ -250,7 +305,7 @@ * gap between reference IDs are acceptable? * further investigation on Apple implementation is needed. */ - for ( j = 0; j < *count; ++j ) + for ( j = 0; j < *count; j++ ) offsets_internal[j] = rdata_pos + ref[j].offset; *offsets = offsets_internal;