Commit e838c37c2c1575eb12116ce6303ffacc72521ce8

Ben Wagner 2022-01-11T11:14:32

[type42] Track how much type42 ttf data is available. Currently `T42_Open_Face` eagerly allocates 12 bytes for the ttf header data which it expects `t42_parse_sfnts` to fill out from /sfnts data. However, there is no guarantee that `t42_parse_sfnts` will actually be called while parsing the type42 data as the /sfnts array may be missing or very short. This is also confusing behavior as it means `T42_Open_Face` is tightly coupled to the implementation of the very distant `t42_parse_sfnts` code which requires at least 12 bytes to already be reserved in `face->ttf_data`. `t42_parse_sfnts` itself eagerly updates `face->ttf_size` to track how much space is reserved for ttf data instead of traking how much data has actually been written into `face->ttf_data`. It will also act strangely in the presense of multiple /sfnts arrays. * src/type42/t42objs.c (T42_Open_Face): ensure `ttf_data` is initialized to NULL. Free `ttf_data` on error. * src/type42/t42parse.c (t42_parse_sfnts): delay setting `ttf_size` and set it to the actual number of bytes read. Ensure `ttf_data` is freed if there are multiple /sfnts arrays or there are any errors.

diff --git a/src/type42/t42objs.c b/src/type42/t42objs.c
index c3e844a..4078777 100644
--- a/src/type42/t42objs.c
+++ b/src/type42/t42objs.c
@@ -44,15 +44,8 @@
 
     parser = &loader.parser;
 
-    /* To handle buggy fonts we don't use `FT_QALLOC` here. */
-    if ( FT_ALLOC( face->ttf_data, 12 ) )
-      goto Exit;
-
-    /* while parsing the font we always update `face->ttf_size' so that */
-    /* even in case of buggy data (which might lead to premature end of */
-    /* scanning without causing an error) the call to `FT_Open_Face' in */
-    /* `T42_Face_Init' passes the correct size                          */
-    face->ttf_size = 12;
+    face->ttf_data = NULL;
+    face->ttf_size = 0;
 
     error = t42_parser_init( parser,
                              face->root.stream,
@@ -153,6 +146,11 @@
 
   Exit:
     t42_loader_done( &loader );
+    if ( error )
+    {
+      FT_FREE(face->ttf_data);
+      face->ttf_size = 0;
+    }
     return error;
   }
 
diff --git a/src/type42/t42parse.c b/src/type42/t42parse.c
index 62b521a..6714074 100644
--- a/src/type42/t42parse.c
+++ b/src/type42/t42parse.c
@@ -538,7 +538,8 @@
     FT_Byte*    limit  = parser->root.limit;
     FT_Error    error;
     FT_Int      num_tables = 0;
-    FT_Long     count;
+    FT_Long     ttf_count;
+    FT_Long     ttf_reserved;
 
     FT_ULong    n, string_size, old_string_size, real_size;
     FT_Byte*    string_buf = NULL;
@@ -546,6 +547,9 @@
 
     T42_Load_Status  status;
 
+    /** There should only be one sfnts array, but free any previous. */
+    FT_FREE( face->ttf_data );
+    face->ttf_size = 0;
 
     /* The format is                                */
     /*                                              */
@@ -574,7 +578,10 @@
     status          = BEFORE_START;
     string_size     = 0;
     old_string_size = 0;
-    count           = 0;
+    ttf_count       = 0;
+    ttf_reserved    = 12;
+    if ( FT_QALLOC( face->ttf_data, ttf_reserved ) )
+      goto Fail;
 
     FT_TRACE2(( "\n" ));
     FT_TRACE2(( "t42_parse_sfnts:\n" ));
@@ -589,6 +596,7 @@
       if ( *cur == ']' )
       {
         parser->root.cursor++;
+        face->ttf_size = ttf_count;
         goto Exit;
       }
 
@@ -684,7 +692,7 @@
       }
 
       FT_TRACE2(( "  PS string size %5lu bytes, offset 0x%08lx (%lu)\n",
-                  string_size, count, count ));
+                  string_size, ttf_count, ttf_count ));
 
       /* The whole TTF is now loaded into `string_buf'.  We are */
       /* checking its contents while copying it to `ttf_data'.  */
@@ -697,45 +705,48 @@
         {
         case BEFORE_START:
           /* load offset table, 12 bytes */
-          if ( count < 12 )
+          if ( ttf_count < 12 )
           {
-            face->ttf_data[count++] = string_buf[n];
+            face->ttf_data[ttf_count++] = string_buf[n];
             continue;
           }
           else
           {
-            num_tables     = 16 * face->ttf_data[4] + face->ttf_data[5];
-            status         = BEFORE_TABLE_DIR;
-            face->ttf_size = 12 + 16 * num_tables;
+            FT_Long ttf_reserved_prev = ttf_reserved;
+
+
+            num_tables   = 16 * face->ttf_data[4] + face->ttf_data[5];
+            status       = BEFORE_TABLE_DIR;
+            ttf_reserved = 12 + 16 * num_tables;
 
             FT_TRACE2(( "  SFNT directory contains %d tables\n",
                         num_tables ));
 
-            if ( (FT_Long)size < face->ttf_size )
+            if ( (FT_Long)size < ttf_reserved )
             {
               FT_ERROR(( "t42_parse_sfnts: invalid data in sfnts array\n" ));
               error = FT_THROW( Invalid_File_Format );
               goto Fail;
             }
 
-            /* To handle bad fonts with an invalid table directory */
-            /* we don't use `FT_QREALLOC` here.                    */
-            if ( FT_REALLOC( face->ttf_data, 12, face->ttf_size ) )
+            if ( FT_QREALLOC( face->ttf_data, ttf_reserved_prev,
+                              ttf_reserved ) )
               goto Fail;
           }
           /* fall through */
 
         case BEFORE_TABLE_DIR:
           /* the offset table is read; read the table directory */
-          if ( count < face->ttf_size )
+          if ( ttf_count < ttf_reserved )
           {
-            face->ttf_data[count++] = string_buf[n];
+            face->ttf_data[ttf_count++] = string_buf[n];
             continue;
           }
           else
           {
             int       i;
             FT_ULong  len;
+            FT_Long ttf_reserved_prev = ttf_reserved;
 
 
             FT_TRACE2(( "\n" ));
@@ -751,7 +762,7 @@
               FT_TRACE2(( "   %4i  0x%08lx (%lu)\n", i, len, len ));
 
               if ( len > size                               ||
-                   face->ttf_size > (FT_Long)( size - len ) )
+                   ttf_reserved > (FT_Long)( size - len ) )
               {
                 FT_ERROR(( "t42_parse_sfnts:"
                            " invalid data in sfnts array\n" ));
@@ -760,35 +771,31 @@
               }
 
               /* Pad to a 4-byte boundary length */
-              face->ttf_size += (FT_Long)( ( len + 3 ) & ~3U );
+              ttf_reserved += (FT_Long)( ( len + 3 ) & ~3U );
             }
+            ttf_reserved += 1;
 
             status = OTHER_TABLES;
 
             FT_TRACE2(( "\n" ));
-            FT_TRACE2(( "  allocating %ld bytes\n", face->ttf_size + 1 ));
+            FT_TRACE2(( "  allocating %ld bytes\n", ttf_reserved ));
             FT_TRACE2(( "\n" ));
 
-            /* To handle bad fonts we don't use `FT_QREALLOC` here:    */
-            /* chances are high that due to incorrect values in the    */
-            /* table directory the computation of `ttf_size` would be  */
-            /* incorrect otherwise, causing run-time errors because of */
-            /* accessing uninitialized memory.                         */
-            if ( FT_REALLOC( face->ttf_data, 12 + 16 * num_tables,
-                             face->ttf_size + 1 ) )
+            if ( FT_QREALLOC( face->ttf_data, ttf_reserved_prev,
+                              ttf_reserved ) )
               goto Fail;
           }
           /* fall through */
 
         case OTHER_TABLES:
           /* all other tables are just copied */
-          if ( count >= face->ttf_size )
+          if ( ttf_count >= ttf_reserved )
           {
             FT_ERROR(( "t42_parse_sfnts: too much binary data\n" ));
             error = FT_THROW( Invalid_File_Format );
             goto Fail;
           }
-          face->ttf_data[count++] = string_buf[n];
+          face->ttf_data[ttf_count++] = string_buf[n];
         }
       }
 
@@ -802,6 +809,11 @@
     parser->root.error = error;
 
   Exit:
+    if ( parser->root.error )
+    {
+      FT_FREE( face->ttf_data );
+      face->ttf_size = 0;
+    }
     if ( allocated )
       FT_FREE( string_buf );
   }