From 730b6d7468327f918472b7780c159d688c68590c Mon Sep 17 00:00:00 2001 From: Werner Lemberg Date: Sat, 19 Sep 2015 12:41:12 +0200 Subject: [PATCH] [sfnt] Improve handling of invalid SFNT table entries (#45987). This patch fixes weaknesses in function `tt_face_load_font_dir'. - It incorrectly assumed that valid tables are always at the beginning. As a consequence, some valid tables after invalid entries (which are ignored) were never seen. - Duplicate table entries (this is, having the same tag) were not rejected. - The number of valid tables was sometimes too large, leading to access of invalid tables. * src/sfnt/ttload.c (check_table_dir): Add argument to return number of valid tables. Add another tracing message. (tt_face_load_font_dir): Only allocate table array for valid entries as returned by `check_table_dir'. Reject duplicate tables and adjust number of valid tables accordingly. --- ChangeLog | 24 ++++++++++ src/sfnt/ttload.c | 117 +++++++++++++++++++++++++++++----------------- 2 files changed, 99 insertions(+), 42 deletions(-) diff --git a/ChangeLog b/ChangeLog index 6d41da6c0..92b5b9b76 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,27 @@ +2015-09-19 Werner Lemberg + + [sfnt] Improve handling of invalid SFNT table entries (#45987). + + This patch fixes weaknesses in function `tt_face_load_font_dir'. + + - It incorrectly assumed that valid tables are always at the + beginning. As a consequence, some valid tables after invalid + entries (which are ignored) were never seen. + + - Duplicate table entries (this is, having the same tag) were not + rejected. + + - The number of valid tables was sometimes too large, leading to + access of invalid tables. + + * src/sfnt/ttload.c (check_table_dir): Add argument to return number + of valid tables. + Add another tracing message. + (tt_face_load_font_dir): Only allocate table array for valid + entries as returned by `check_table_dir'. + Reject duplicate tables and adjust number of valid tables + accordingly. + 2015-09-19 Werner Lemberg [pcf] Improve `FT_ABS' fix from 2015-09-17 (#45999). diff --git a/src/sfnt/ttload.c b/src/sfnt/ttload.c index ad2975de0..c1bd7f0c8 100644 --- a/src/sfnt/ttload.c +++ b/src/sfnt/ttload.c @@ -151,7 +151,8 @@ /* Here, we */ /* */ - /* - check that `num_tables' is valid (and adjust it if necessary) */ + /* - check that `num_tables' is valid (and adjust it if necessary); */ + /* also return the number of valid table entries */ /* */ /* - look for a `head' table, check its size, and parse it to check */ /* whether its `magic' field is correctly set */ @@ -167,7 +168,8 @@ /* */ static FT_Error check_table_dir( SFNT_Header sfnt, - FT_Stream stream ) + FT_Stream stream, + FT_UShort* valid ) { FT_Error error; FT_UShort nn, valid_entries = 0; @@ -209,7 +211,10 @@ /* we ignore invalid tables */ if ( table.Offset > stream->size ) + { + FT_TRACE2(( "check_table_dir: table entry %d invalid\n", nn )); continue; + } else if ( table.Length > stream->size - table.Offset ) { /* Some tables have such a simple structure that clipping its */ @@ -273,11 +278,11 @@ has_meta = 1; } - sfnt->num_tables = valid_entries; + *valid = valid_entries; - if ( sfnt->num_tables == 0 ) + if ( !valid_entries ) { - FT_TRACE2(( "check_table_dir: no tables found\n" )); + FT_TRACE2(( "check_table_dir: no valid tables found\n" )); error = FT_THROW( Unknown_File_Format ); goto Exit; } @@ -333,8 +338,7 @@ SFNT_HeaderRec sfnt; FT_Error error; FT_Memory memory = stream->memory; - TT_TableRec* entry; - FT_Int nn; + FT_UShort nn, valid_entries; static const FT_Frame_Field offset_table_fields[] = { @@ -375,85 +379,114 @@ if ( sfnt.format_tag != TTAG_OTTO ) { /* check first */ - error = check_table_dir( &sfnt, stream ); + error = check_table_dir( &sfnt, stream, &valid_entries ); if ( error ) { FT_TRACE2(( "tt_face_load_font_dir:" " invalid table directory for TrueType\n" )); - goto Exit; } } + else + valid_entries = sfnt.num_tables; - face->num_tables = sfnt.num_tables; + face->num_tables = valid_entries; face->format_tag = sfnt.format_tag; if ( FT_QNEW_ARRAY( face->dir_tables, face->num_tables ) ) goto Exit; - if ( FT_STREAM_SEEK( sfnt.offset + 12 ) || - FT_FRAME_ENTER( face->num_tables * 16L ) ) + if ( FT_STREAM_SEEK( sfnt.offset + 12 ) || + FT_FRAME_ENTER( sfnt.num_tables * 16L ) ) goto Exit; - entry = face->dir_tables; - FT_TRACE2(( "\n" " tag offset length checksum\n" " ----------------------------------\n" )); + valid_entries = 0; for ( nn = 0; nn < sfnt.num_tables; nn++ ) { - entry->Tag = FT_GET_TAG4(); - entry->CheckSum = FT_GET_ULONG(); - entry->Offset = FT_GET_ULONG(); - entry->Length = FT_GET_ULONG(); + TT_TableRec entry; + FT_UShort i; + FT_Bool duplicate; + + + entry.Tag = FT_GET_TAG4(); + entry.CheckSum = FT_GET_ULONG(); + entry.Offset = FT_GET_ULONG(); + entry.Length = FT_GET_ULONG(); /* ignore invalid tables that can't be sanitized */ - if ( entry->Offset > stream->size ) + if ( entry.Offset > stream->size ) continue; - else if ( entry->Length > stream->size - entry->Offset ) + else if ( entry.Length > stream->size - entry.Offset ) { - if ( entry->Tag == TTAG_hmtx || - entry->Tag == TTAG_vmtx ) + if ( entry.Tag == TTAG_hmtx || + entry.Tag == TTAG_vmtx ) { #ifdef FT_DEBUG_LEVEL_TRACE - FT_ULong old_length = entry->Length; + FT_ULong old_length = entry.Length; #endif /* make metrics table length a multiple of 4 */ - entry->Length = ( stream->size - entry->Offset ) & ~3U; + entry.Length = ( stream->size - entry.Offset ) & ~3U; FT_TRACE2(( " %c%c%c%c %08lx %08lx %08lx" - " (sanitized; original length %08lx)\n", - (FT_Char)( entry->Tag >> 24 ), - (FT_Char)( entry->Tag >> 16 ), - (FT_Char)( entry->Tag >> 8 ), - (FT_Char)( entry->Tag ), - entry->Offset, - entry->Length, - entry->CheckSum, + " (sanitized; original length %08lx)", + (FT_Char)( entry.Tag >> 24 ), + (FT_Char)( entry.Tag >> 16 ), + (FT_Char)( entry.Tag >> 8 ), + (FT_Char)( entry.Tag ), + entry.Offset, + entry.Length, + entry.CheckSum, old_length )); - entry++; } else continue; } +#ifdef FT_DEBUG_LEVEL_TRACE + else + FT_TRACE2(( " %c%c%c%c %08lx %08lx %08lx", + (FT_Char)( entry.Tag >> 24 ), + (FT_Char)( entry.Tag >> 16 ), + (FT_Char)( entry.Tag >> 8 ), + (FT_Char)( entry.Tag ), + entry.Offset, + entry.Length, + entry.CheckSum )); +#endif + + /* ignore duplicate tables – the first one wins */ + duplicate = 0; + for ( i = 0; i < valid_entries; i++ ) + { + if ( face->dir_tables[i].Tag == entry.Tag ) + { + duplicate = 1; + break; + } + } + if ( duplicate ) + { + FT_TRACE2(( " (duplicate, ignored)\n" )); + continue; + } else { - FT_TRACE2(( " %c%c%c%c %08lx %08lx %08lx\n", - (FT_Char)( entry->Tag >> 24 ), - (FT_Char)( entry->Tag >> 16 ), - (FT_Char)( entry->Tag >> 8 ), - (FT_Char)( entry->Tag ), - entry->Offset, - entry->Length, - entry->CheckSum )); - entry++; + FT_TRACE2(( "\n" )); + + /* we finally have a valid entry */ + face->dir_tables[valid_entries++] = entry; } } + /* final adjustment to number of tables */ + face->num_tables = valid_entries; + FT_FRAME_EXIT(); FT_TRACE2(( "table directory loaded\n\n" ));