diff --git a/ChangeLog b/ChangeLog index 4694aea74..8c048c28b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,26 @@ +2019-03-31 Armin Hasitzka + + [cff] Fix boundary checks. + + 642bc7590c701c8cd35a9f60fa899cfa518b17ff introduced dynamically + allocated memory when parsing CFF files with the "old" engine. Bounds + checks have never been updated, however, leading to pointless + comparisons of pointers in some cases. This commit presents a + solution for bounds checks in the CFF module with an extended logic + for the "old" engine while staying as concise as possible for the + "new" one. + + * src/cff/cffparse.h: Introduce the struct `CFF_T2_StringRec' and + the additional field `t2_strings' within `CFF_ParserRec'. + + * src/cff/cffparse.c (cff_parser_within_limits): Move all boundary + checks into this new function and update the rest of `cffparse.c' to + use it. + + Reported as + + https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=12137 + 2019-03-20 Werner Lemberg [autofit] Fix Mongolian blue zone characters. diff --git a/src/cff/cffparse.c b/src/cff/cffparse.c index fa806f1a9..008752c3a 100644 --- a/src/cff/cffparse.c +++ b/src/cff/cffparse.c @@ -77,6 +77,23 @@ } +#ifdef CFF_CONFIG_OPTION_OLD_ENGINE + static void + finalize_t2_strings( FT_Memory memory, + void* data, + void* user ) + { + CFF_T2_String t2 = (CFF_T2_String)data; + + + FT_UNUSED( user ); + + memory->free( memory, t2->start ); + memory->free( memory, data ); + } +#endif /* CFF_CONFIG_OPTION_OLD_ENGINE */ + + FT_LOCAL_DEF( void ) cff_parser_done( CFF_Parser parser ) { @@ -84,13 +101,65 @@ FT_FREE( parser->stack ); + +#ifdef CFF_CONFIG_OPTION_OLD_ENGINE + FT_List_Finalize( &parser->t2_strings, + finalize_t2_strings, + memory, + NULL ); +#endif + } + + + /* Assuming `first >= last'. */ + + static FT_Error + cff_parser_within_limits( CFF_Parser parser, + FT_Byte* first, + FT_Byte* last ) + { +#ifndef CFF_CONFIG_OPTION_OLD_ENGINE + + /* Fast path for regular FreeType builds with the "new" engine; */ + /* `first >= parser->start' can be assumed. */ + + FT_UNUSED( first ); + + return last < parser->limit ? FT_Err_Ok : FT_THROW( Invalid_Argument ); + +#else /* CFF_CONFIG_OPTION_OLD_ENGINE */ + + FT_ListNode node; + + + if ( first >= parser->start && + last < parser->limit ) + return FT_Err_Ok; + + node = parser->t2_strings.head; + + while ( node ) + { + CFF_T2_String t2 = (CFF_T2_String)node->data; + + + if ( first >= t2->start && + last < t2->limit ) + return FT_Err_Ok; + + node = node->next; + } + + return FT_THROW( Invalid_Argument ); + +#endif /* CFF_CONFIG_OPTION_OLD_ENGINE */ } /* read an integer */ static FT_Long - cff_parse_integer( FT_Byte* start, - FT_Byte* limit ) + cff_parse_integer( CFF_Parser parser, + FT_Byte* start ) { FT_Byte* p = start; FT_Int v = *p++; @@ -99,14 +168,14 @@ if ( v == 28 ) { - if ( p + 2 > limit ) + if ( cff_parser_within_limits( parser, p, p + 1 ) ) goto Bad; val = (FT_Short)( ( (FT_UShort)p[0] << 8 ) | p[1] ); } else if ( v == 29 ) { - if ( p + 4 > limit ) + if ( cff_parser_within_limits( parser, p, p + 3 ) ) goto Bad; val = (FT_Long)( ( (FT_ULong)p[0] << 24 ) | @@ -120,14 +189,14 @@ } else if ( v < 251 ) { - if ( p + 1 > limit ) + if ( cff_parser_within_limits( parser, p, p ) ) goto Bad; val = ( v - 247 ) * 256 + p[0] + 108; } else { - if ( p + 1 > limit ) + if ( cff_parser_within_limits( parser, p, p ) ) goto Bad; val = -( v - 251 ) * 256 - p[0] - 108; @@ -176,10 +245,10 @@ /* read a real */ static FT_Fixed - cff_parse_real( FT_Byte* start, - FT_Byte* limit, - FT_Long power_ten, - FT_Long* scaling ) + cff_parse_real( CFF_Parser parser, + FT_Byte* start, + FT_Long power_ten, + FT_Long* scaling ) { FT_Byte* p = start; FT_Int nib; @@ -214,7 +283,7 @@ p++; /* Make sure we don't read past the end. */ - if ( p >= limit ) + if ( cff_parser_within_limits( parser, p, p ) ) goto Bad; } @@ -251,7 +320,7 @@ p++; /* Make sure we don't read past the end. */ - if ( p >= limit ) + if ( cff_parser_within_limits( parser, p, p ) ) goto Bad; } @@ -290,7 +359,7 @@ p++; /* Make sure we don't read past the end. */ - if ( p >= limit ) + if ( cff_parser_within_limits( parser, p, p ) ) goto Bad; } @@ -457,7 +526,7 @@ if ( **d == 30 ) { /* binary-coded decimal is truncated to integer */ - return cff_parse_real( *d, parser->limit, 0, NULL ) >> 16; + return cff_parse_real( parser, *d, 0, NULL ) >> 16; } else if ( **d == 255 ) @@ -483,7 +552,7 @@ } else - return cff_parse_integer( *d, parser->limit ); + return cff_parse_integer( parser, *d ); } @@ -494,10 +563,10 @@ FT_Long scaling ) { if ( **d == 30 ) - return cff_parse_real( *d, parser->limit, scaling, NULL ); + return cff_parse_real( parser, *d, scaling, NULL ); else { - FT_Long val = cff_parse_integer( *d, parser->limit ); + FT_Long val = cff_parse_integer( parser, *d ); if ( scaling ) @@ -562,14 +631,14 @@ FT_ASSERT( scaling ); if ( **d == 30 ) - return cff_parse_real( *d, parser->limit, 0, scaling ); + return cff_parse_real( parser, *d, 0, scaling ); else { FT_Long number; FT_Int integer_length; - number = cff_parse_integer( d[0], d[1] ); + number = cff_parse_integer( parser, d[0] ); if ( number > 0x7FFFL ) { @@ -1122,18 +1191,6 @@ #endif /* FT_DEBUG_LEVEL_TRACE */ -#ifdef CFF_CONFIG_OPTION_OLD_ENGINE - static void - destruct_t2s_item( FT_Memory memory, - void* data, - void* user ) - { - FT_UNUSED( user ); - memory->free( memory, data ); - } -#endif /* CFF_CONFIG_OPTION_OLD_ENGINE */ - - FT_LOCAL_DEF( FT_Error ) cff_parser_run( CFF_Parser parser, FT_Byte* start, @@ -1147,11 +1204,6 @@ FT_Library library = parser->library; FT_Memory memory = library->memory; - - FT_ListRec t2s; - - - FT_ZERO( &t2s ); #endif parser->top = parser->stack; @@ -1212,9 +1264,11 @@ FT_Byte* charstring_base; FT_ULong charstring_len; - FT_Fixed* stack; - FT_ListNode node; - FT_Byte* q; + FT_Fixed* stack; + FT_ListNode node; + CFF_T2_String t2; + size_t t2_size; + FT_Byte* q; charstring_base = ++p; @@ -1261,16 +1315,26 @@ if ( !node ) goto Out_Of_Memory_Error; + FT_List_Add( &parser->t2_strings, node ); + + t2 = (CFF_T2_String)memory->alloc( memory, + sizeof ( CFF_T2_StringRec ) ); + if ( !t2 ) + goto Out_Of_Memory_Error; + + node->data = t2; + /* `5' is the conservative upper bound of required bytes per stack */ /* element. */ - q = (FT_Byte*)memory->alloc( memory, - 5 * ( decoder.top - decoder.stack ) ); + + t2_size = 5 * ( decoder.top - decoder.stack ); + + q = (FT_Byte*)memory->alloc( memory, t2_size ); if ( !q ) goto Out_Of_Memory_Error; - node->data = q; - - FT_List_Add( &t2s, node ); + t2->start = q; + t2->limit = q + t2_size; stack = decoder.stack; @@ -1531,9 +1595,6 @@ } /* while ( p < limit ) */ Exit: -#ifdef CFF_CONFIG_OPTION_OLD_ENGINE - FT_List_Finalize( &t2s, destruct_t2s_item, memory, NULL ); -#endif return error; #ifdef CFF_CONFIG_OPTION_OLD_ENGINE diff --git a/src/cff/cffparse.h b/src/cff/cffparse.h index bac32f944..4e74709a2 100644 --- a/src/cff/cffparse.h +++ b/src/cff/cffparse.h @@ -60,6 +60,10 @@ FT_BEGIN_HEADER FT_Byte** top; FT_UInt stackSize; /* allocated size */ +#ifdef CFF_CONFIG_OPTION_OLD_ENGINE + FT_ListRec t2_strings; +#endif /* CFF_CONFIG_OPTION_OLD_ENGINE */ + FT_UInt object_code; void* object; @@ -130,6 +134,15 @@ FT_BEGIN_HEADER FT_END_HEADER +#ifdef CFF_CONFIG_OPTION_OLD_ENGINE + typedef struct CFF_T2_String_ + { + FT_Byte* start; + FT_Byte* limit; + + } CFF_T2_StringRec, *CFF_T2_String; +#endif /* CFF_CONFIG_OPTION_OLD_ENGINE */ + #endif /* CFFPARSE_H_ */