Commit 11975fe9f6ed92b7630698e599b936a0186d965d

Ben Wagner 2020-02-29T20:18:00

Fix state of `FT_Face' for buggy `gvar' tables (#57923). By resetting the blend as implemented with this commit fonts with invalid `gvar' tables may keep calling into `ft_var_load_gvar' from `tt_set_mm_blend' and failing, but the font was invalid anyway and we want to keep seeing the failure in `tt_set_mm_blend'. * src/truetype/ttgxvar.c (ft_var_load_gvar): Calculate length of offset array once. Allocate arrays after `FT_FRAME_ENTER' (extra check before allocating and avoid needing to free array later if error entering frame). Always call `FT_FRAME_EXIT'. Consistently set counts immediately after array initialized. Reset the blend (particularly `blend->glyphoffsets') on failure.

diff --git a/ChangeLog b/ChangeLog
index 2886b70..6bbb957 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,21 @@
+2020-02-29  Ben Wagner  <bungeman@google.com>
+
+	[truetype] Fix state of `FT_Face' for buggy `gvar' tables (#57923).
+
+	By resetting the blend as implemented with this commit fonts with
+	invalid `gvar' tables may keep calling into `ft_var_load_gvar' from
+	`tt_set_mm_blend' and failing, but the font was invalid anyway and
+	we want to keep seeing the failure in `tt_set_mm_blend'.
+
+	* src/truetype/ttgxvar.c (ft_var_load_gvar): Calculate length of
+	offset array once.
+	Allocate arrays after `FT_FRAME_ENTER' (extra check before
+	allocating and avoid needing to free array later if error entering
+	frame).
+	Always call `FT_FRAME_EXIT'.
+	Consistently set counts immediately after array initialized.
+	Reset the blend (particularly `blend->glyphoffsets') on failure.
+
 2020-03-01  Nikhil Ramakrishnan  <ramakrishnan.nikhil@gmail.com>
 
 	[docs] Update docwriter stylesheet.
diff --git a/src/truetype/ttgxvar.c b/src/truetype/ttgxvar.c
index ce6b62e..110f24a 100644
--- a/src/truetype/ttgxvar.c
+++ b/src/truetype/ttgxvar.c
@@ -1470,6 +1470,7 @@
     FT_ULong      table_len;
     FT_ULong      gvar_start;
     FT_ULong      offsetToData;
+    FT_ULong      offsets_len;
     GX_GVar_Head  gvar_head;
 
     static const FT_Frame_Field  gvar_fields[] =
@@ -1530,9 +1531,13 @@
       goto Exit;
     }
 
-    /* rough sanity check: offsets can be either 2 or 4 bytes */
-    if ( (FT_ULong)gvar_head.glyphCount *
-           ( ( gvar_head.flags & 1 ) ? 4 : 2 ) > table_len )
+    /* offsets can be either 2 or 4 bytes                  */
+    /* (one more offset than glyphs, to mark size of last) */
+    offsets_len = ( gvar_head.glyphCount + 1 ) *
+                  ( ( gvar_head.flags & 1 ) ? 4L : 2L );
+
+    /* rough sanity check */
+    if (offsets_len > table_len )
     {
       FT_TRACE1(( "ft_var_load_gvar: invalid number of glyphs\n" ));
       error = FT_THROW( Invalid_Table );
@@ -1549,19 +1554,19 @@
                 gvar_head.globalCoordCount,
                 gvar_head.globalCoordCount == 1 ? "" : "s" ));
 
-    if ( FT_NEW_ARRAY( blend->glyphoffsets, gvar_head.glyphCount + 1 ) )
+    if ( FT_FRAME_ENTER( offsets_len ) )
       goto Exit;
 
+    /* offsets (one more offset than glyphs, to mark size of last) */
+    if ( FT_NEW_ARRAY( blend->glyphoffsets, gvar_head.glyphCount + 1 ) )
+      goto Fail2;
+
     if ( gvar_head.flags & 1 )
     {
       FT_ULong  limit      = gvar_start + table_len;
       FT_ULong  max_offset = 0;
 
 
-      /* long offsets (one more offset than glyphs, to mark size of last) */
-      if ( FT_FRAME_ENTER( ( gvar_head.glyphCount + 1 ) * 4L ) )
-        goto Exit;
-
       for ( i = 0; i <= gvar_head.glyphCount; i++ )
       {
         blend->glyphoffsets[i] = offsetToData + FT_GET_ULONG();
@@ -1592,10 +1597,6 @@
       FT_ULong  max_offset = 0;
 
 
-      /* short offsets (one more offset than glyphs, to mark size of last) */
-      if ( FT_FRAME_ENTER( ( gvar_head.glyphCount + 1 ) * 2L ) )
-        goto Exit;
-
       for ( i = 0; i <= gvar_head.glyphCount; i++ )
       {
         blend->glyphoffsets[i] = offsetToData + FT_GET_USHORT() * 2;
@@ -1621,12 +1622,10 @@
       }
     }
 
-    FT_FRAME_EXIT();
-    if ( error )
-      goto Exit;
-
     blend->gv_glyphcnt = gvar_head.glyphCount;
 
+    FT_FRAME_EXIT();
+
     if ( gvar_head.globalCoordCount != 0 )
     {
       if ( FT_STREAM_SEEK( gvar_start + gvar_head.offsetToCoord ) ||
@@ -1635,16 +1634,14 @@
       {
         FT_TRACE2(( "ft_var_load_gvar:"
                     " glyph variation shared tuples missing\n" ));
-        goto Exit;
+        goto Fail;
       }
 
       if ( FT_NEW_ARRAY( blend->tuplecoords,
                          gvar_head.axisCount * gvar_head.globalCoordCount ) )
-        goto Exit;
-
-      blend->tuplecount = gvar_head.globalCoordCount;
+        goto Fail2;
 
-      for ( i = 0; i < blend->tuplecount; i++ )
+      for ( i = 0; i < gvar_head.globalCoordCount; i++ )
       {
         FT_TRACE5(( "  [ " ));
         for ( j = 0; j < (FT_UInt)gvar_head.axisCount; j++ )
@@ -1657,6 +1654,8 @@
         FT_TRACE5(( "]\n" ));
       }
 
+      blend->tuplecount = gvar_head.globalCoordCount;
+
       FT_TRACE5(( "\n" ));
 
       FT_FRAME_EXIT();
@@ -1664,6 +1663,14 @@
 
   Exit:
     return error;
+
+  Fail2:
+    FT_FRAME_EXIT();
+
+  Fail:
+    FT_FREE( blend->glyphoffsets );
+    blend->gv_glyphcnt = 0;
+    goto Exit;
   }