[freetype2] master 730b6d7: [sfnt] Improve handling of invalid SFNT table entries (#45987).

[Werner LEMBERG]

branch: master
commit 730b6d7468327f918472b7780c159d688c68590c
Author: Werner Lemberg <address@hidden>
Commit: Werner Lemberg <address@hidden>

    [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 6d41da6..92b5b9b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,29 @@
 2015-09-19  Werner Lemberg  <address@hidden>
 
+       [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  <address@hidden>
+
        [pcf] Improve `FT_ABS' fix from 2015-09-17 (#45999).
 
        * src/pcf/pcfread.c (pcf_load_font): Do first the cast to FT_Short,
diff --git a/src/sfnt/ttload.c b/src/sfnt/ttload.c
index ad2975d..c1bd7f0 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" ));