[freetype2] master 369d8be: [truetype] Prevent glyph program state from persisting.

[Werner Lemberg]

branch: master
commit 369d8be97f537c56f2dbd1d3bd23aba74dc24cea
Author: Ben Wagner <bungeman@chromium.org>
Commit: Werner Lemberg <wl@gnu.org>

    [truetype] Prevent glyph program state from persisting.
    
    `FDEF` instructions are specified as allowed only in 'prep' or
    'fpgm'.  FreeType has attempted to prevent their use in the glyph
    program, but they were still allowed in glyph programs if defined in
    a function defined in 'prep' or 'fpgm' and called from the glyph
    program.
    
    Similarly, `IDEF` instructions are specified not to be able to
    modify any existing instruction.  FreeType has attempted to prevent
    their use in the glyph program, but they can still be used like
    `FDEF`.
    
    This change stores the initial bytecode range type and disallows the
    use of `FDEF` and `IDEF` while running the glyph program.
    
    Most other state is copied from the `TT_Size` into the execution
    context.  However, it is possible for a glyph program to use `WS` to
    write to the storage area or `WCVTP`, `WCVTF`, and `DELTAC[123]` to
    write to the control value table.
    
    Allowing any change to the global state from the glyph program is
    problematic as the outlines of any given glyph may change based on
    the order the glyphs are loaded or even how many times they are
    loaded.  There exist fonts that write to the storage area or the
    control value table in the glyph program, so their use should not be
    an error.
    
    Possible solutions to using these in the glyph program are
    
      * ignore the writes;
      * value-level copy on write, discard modified values when finished;
      * array-level copy on write, discard the copy when finished;
      * array-level copy up-front.
    
    Ignoring the writes may break otherwise good uses.  A full copy
    up-front was implemented, but was quite heavy as even well behaved
    fonts required a full copy and the memory management that goes along
    with it.  Value-level copy on write could use less memory but
    requires a great deal more record keeping and complexity.  This
    change implements array-level copy on write.  If any attempt is made
    to write to the control value table or the storage area when the
    initial bytecode range was in a glyph program, the relevant array
    will be copied to a designated storage area and the copy used for
    the rest of the glyph program's execution.
    
    * src/truetype/ttinterp.h (TT_ExecContextRec): New fields
    `iniRange`, `glyfCvtSize`, `glyfCvt`, `origCvt`, `glyfStoreSize`,
    `glyfStorage`, and `origStorage`.
    
    * src/truetype/ttinterp.c (Modify_CVT_Check): New function to handle
    `exc->glyfCvt`.
    (Write_CVT, Write_CVT_Stretched, Move_CVT, Move_CVT_Stretched): Use
    it.
    (Ins_WS): Handle `exc->glyfStorage`.
    (Ins_FDEF, Ins_IDEF): Updated.
    (TT_RunIns): Updated.
    (TT_Done_Context): Free 'glyf' CVT working and storage area.
    (TT_Load_Context): Fix/add casts.
    
    * src/truetype/ttgload.c (TT_Load_Simple_Glyph): Fix cast.
---
 ChangeLog               | 66 ++++++++++++++++++++++++++++++++++-
 src/truetype/ttgload.c  |  2 +-
 src/truetype/ttinterp.c | 93 +++++++++++++++++++++++++++++++++++++++++++++----
 src/truetype/ttinterp.h | 11 ++++--
 4 files changed, 162 insertions(+), 10 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index ec0d132..0e599b9 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,67 @@
+2021-04-02  Ben Wagner  <bungeman@chromium.org>
+
+       [truetype] Prevent glyph program state from persisting.
+
+       `FDEF` instructions are specified as allowed only in 'prep' or
+       'fpgm'.  FreeType has attempted to prevent their use in the glyph
+       program, but they were still allowed in glyph programs if defined in
+       a function defined in 'prep' or 'fpgm' and called from the glyph
+       program.
+
+       Similarly, `IDEF` instructions are specified not to be able to
+       modify any existing instruction.  FreeType has attempted to prevent
+       their use in the glyph program, but they can still be used like
+       `FDEF`.
+
+       This change stores the initial bytecode range type and disallows the
+       use of `FDEF` and `IDEF` while running the glyph program.
+
+       Most other state is copied from the `TT_Size` into the execution
+       context.  However, it is possible for a glyph program to use `WS` to
+       write to the storage area or `WCVTP`, `WCVTF`, and `DELTAC[123]` to
+       write to the control value table.
+
+       Allowing any change to the global state from the glyph program is
+       problematic as the outlines of any given glyph may change based on
+       the order the glyphs are loaded or even how many times they are
+       loaded.  There exist fonts that write to the storage area or the
+       control value table in the glyph program, so their use should not be
+       an error.
+
+       Possible solutions to using these in the glyph program are
+
+         * ignore the writes;
+         * value-level copy on write, discard modified values when finished;
+         * array-level copy on write, discard the copy when finished;
+         * array-level copy up-front.
+
+       Ignoring the writes may break otherwise good uses.  A full copy
+       up-front was implemented, but was quite heavy as even well behaved
+       fonts required a full copy and the memory management that goes along
+       with it.  Value-level copy on write could use less memory but
+       requires a great deal more record keeping and complexity.  This
+       change implements array-level copy on write.  If any attempt is made
+       to write to the control value table or the storage area when the
+       initial bytecode range was in a glyph program, the relevant array
+       will be copied to a designated storage area and the copy used for
+       the rest of the glyph program's execution.
+
+       * src/truetype/ttinterp.h (TT_ExecContextRec): New fields
+       `iniRange`, `glyfCvtSize`, `glyfCvt`, `origCvt`, `glyfStoreSize`,
+       `glyfStorage`, and `origStorage`.
+
+       * src/truetype/ttinterp.c (Modify_CVT_Check): New function to handle
+       `exc->glyfCvt`.
+       (Write_CVT, Write_CVT_Stretched, Move_CVT, Move_CVT_Stretched): Use
+       it.
+       (Ins_WS): Handle `exc->glyfStorage`.
+       (Ins_FDEF, Ins_IDEF): Updated.
+       (TT_RunIns): Updated.
+       (TT_Done_Context): Free 'glyf' CVT working and storage area.
+       (TT_Load_Context): Fix/add casts.
+
+       * src/truetype/ttgload.c (TT_Load_Simple_Glyph): Fix cast.
+
 2021-03-30  Dominik Röttsches  <drott@chromium.org>
 
        [sfnt] Check validity of pointer location of `read_color_line`.
@@ -277,7 +341,7 @@
 
        [sfnt] Provide optional root transform for 'COLR' v1 glyph graph.
 
-        * include/freetype/freetype.h (FT_Get_Color_Glyph_Paint):
+       * include/freetype/freetype.h (FT_Get_Color_Glyph_Paint):
        Additional function argument `root_transform` to control whether
        root transform should be returned.
        (FT_OpaquePaint): Additional tracking field to denote whether
diff --git a/src/truetype/ttgload.c b/src/truetype/ttgload.c
index 61f7972..b490b84 100644
--- a/src/truetype/ttgload.c
+++ b/src/truetype/ttgload.c
@@ -458,7 +458,7 @@
                           (void*)&load->exec->glyphIns,
                           n_ins );
 
-      load->exec->glyphSize = (FT_UShort)tmp;
+      load->exec->glyphSize = (FT_UInt)tmp;
       if ( error )
         return error;
 
diff --git a/src/truetype/ttinterp.c b/src/truetype/ttinterp.c
index 482c0a5..ee21a1f 100644
--- a/src/truetype/ttinterp.c
+++ b/src/truetype/ttinterp.c
@@ -251,6 +251,14 @@
     FT_FREE( exec->stack );
     exec->stackSize = 0;
 
+    /* free glyf cvt working area */
+    FT_FREE( exec->glyfCvt );
+    exec->glyfCvtSize = 0;
+
+    /* free glyf storage working area */
+    FT_FREE( exec->glyfStorage );
+    exec->glyfStoreSize = 0;
+
     /* free call stack */
     FT_FREE( exec->callStack );
     exec->callSize = 0;
@@ -464,13 +472,13 @@
     if ( error )
       return error;
 
-    tmp = exec->glyphSize;
+    tmp = (FT_ULong)exec->glyphSize;
     error = Update_Max( exec->memory,
                         &tmp,
                         sizeof ( FT_Byte ),
                         (void*)&exec->glyphIns,
                         maxp->maxSizeOfInstructions );
-    exec->glyphSize = (FT_UShort)tmp;
+    exec->glyphSize = (FT_UInt)tmp;
     if ( error )
       return error;
 
@@ -1572,11 +1580,36 @@
   }
 
 
+  static void
+  Modify_CVT_Check( TT_ExecContext  exc )
+  {
+    /* TT_RunIns sets origCvt and restores cvt to origCvt when done. */
+    if ( exc->iniRange == tt_coderange_glyph &&
+         exc->cvt == exc->origCvt            )
+    {
+      exc->error = Update_Max( exc->memory,
+                               &exc->glyfCvtSize,
+                               sizeof ( FT_Long ),
+                               (void*)&exc->glyfCvt,
+                               exc->cvtSize );
+      if ( exc->error )
+        return;
+
+      FT_ARRAY_COPY( exc->glyfCvt, exc->cvt, exc->glyfCvtSize );
+      exc->cvt = exc->glyfCvt;
+    }
+  }
+
+
   FT_CALLBACK_DEF( void )
   Write_CVT( TT_ExecContext  exc,
              FT_ULong        idx,
              FT_F26Dot6      value )
   {
+    Modify_CVT_Check( exc );
+    if ( exc->error )
+      return;
+
     exc->cvt[idx] = value;
   }
 
@@ -1586,6 +1619,10 @@
                        FT_ULong        idx,
                        FT_F26Dot6      value )
   {
+    Modify_CVT_Check( exc );
+    if ( exc->error )
+      return;
+
     exc->cvt[idx] = FT_DivFix( value, Current_Ratio( exc ) );
   }
 
@@ -1595,6 +1632,10 @@
             FT_ULong        idx,
             FT_F26Dot6      value )
   {
+    Modify_CVT_Check( exc );
+    if ( exc->error )
+      return;
+
     exc->cvt[idx] = ADD_LONG( exc->cvt[idx], value );
   }
 
@@ -1604,6 +1645,10 @@
                       FT_ULong        idx,
                       FT_F26Dot6      value )
   {
+    Modify_CVT_Check( exc );
+    if ( exc->error )
+      return;
+
     exc->cvt[idx] = ADD_LONG( exc->cvt[idx],
                               FT_DivFix( value, Current_Ratio( exc ) ) );
   }
@@ -3125,7 +3170,30 @@
         ARRAY_BOUND_ERROR;
     }
     else
+    {
+      /* TT_RunIns sets origStorage and restores storage to origStorage */
+      /* when done.                                                     */
+      if ( exc->iniRange == tt_coderange_glyph &&
+           exc->storage == exc->origStorage    )
+      {
+        FT_ULong  tmp = (FT_ULong)exc->glyfStoreSize;
+
+
+        exc->error = Update_Max( exc->memory,
+                                 &tmp,
+                                 sizeof ( FT_Long ),
+                                 (void*)&exc->glyfStorage,
+                                 exc->storeSize );
+        exc->glyfStoreSize = (FT_UShort)tmp;
+        if ( exc->error )
+          return;
+
+        FT_ARRAY_COPY( exc->glyfStorage, exc->storage, exc->glyfStoreSize );
+        exc->storage = exc->glyfStorage;
+      }
+
       exc->storage[I] = args[1];
+    }
   }
 
 
@@ -3697,7 +3765,7 @@
 
 
     /* FDEF is only allowed in `prep' or `fpgm' */
-    if ( exc->curRange == tt_coderange_glyph )
+    if ( exc->iniRange == tt_coderange_glyph )
     {
       exc->error = FT_THROW( DEF_In_Glyf_Bytecode );
       return;
@@ -4133,7 +4201,7 @@
 
 
     /* we enable IDEF only in `prep' or `fpgm' */
-    if ( exc->curRange == tt_coderange_glyph )
+    if ( exc->iniRange == tt_coderange_glyph )
     {
       exc->error = FT_THROW( DEF_In_Glyf_Bytecode );
       return;
@@ -7842,6 +7910,10 @@
       exc->func_move_cvt  = Move_CVT;
     }
 
+    exc->origCvt     = exc->cvt;
+    exc->origStorage = exc->storage;
+    exc->iniRange    = exc->curRange;
+
     Compute_Funcs( exc );
     Compute_Round( exc, (FT_Byte)exc->GS.round_state );
 
@@ -8566,8 +8638,10 @@
 
       /* increment instruction counter and check if we didn't */
       /* run this program for too long (e.g. infinite loops). */
-      if ( ++ins_counter > TT_CONFIG_OPTION_MAX_RUNNABLE_OPCODES )
-        return FT_THROW( Execution_Too_Long );
+      if ( ++ins_counter > TT_CONFIG_OPTION_MAX_RUNNABLE_OPCODES ) {
+        exc->error = FT_THROW( Execution_Too_Long );
+        goto LErrorLabel_;
+      }
 
     LSuiteLabel_:
       if ( exc->IP >= exc->codeSize )
@@ -8586,6 +8660,10 @@
     FT_TRACE4(( "  %ld instruction%s executed\n",
                 ins_counter,
                 ins_counter == 1 ? "" : "s" ));
+
+    exc->cvt     = exc->origCvt;
+    exc->storage = exc->origStorage;
+
     return FT_Err_Ok;
 
   LErrorCodeOverflow_:
@@ -8595,6 +8673,9 @@
     if ( exc->error && !exc->instruction_trap )
       FT_TRACE1(( "  The interpreter returned error 0x%x\n", exc->error ));
 
+    exc->cvt     = exc->origCvt;
+    exc->storage = exc->origStorage;
+
     return exc->error;
   }
 
diff --git a/src/truetype/ttinterp.h b/src/truetype/ttinterp.h
index deb9e40..98bd8cb 100644
--- a/src/truetype/ttinterp.h
+++ b/src/truetype/ttinterp.h
@@ -175,6 +175,7 @@ FT_BEGIN_HEADER
 
     TT_GraphicsState   GS;         /* current graphics state */
 
+    FT_Int             iniRange;  /* initial code range number   */
     FT_Int             curRange;  /* current code range number   */
     FT_Byte*           code;      /* current code range          */
     FT_Long            IP;        /* current instruction pointer */
@@ -187,6 +188,9 @@ FT_BEGIN_HEADER
                                   /* increment IP after ins. exec */
     FT_ULong           cvtSize;
     FT_Long*           cvt;
+    FT_ULong           glyfCvtSize;
+    FT_Long*           glyfCvt;   /* cvt working copy for glyph */
+    FT_Long*           origCvt;
 
     FT_UInt            glyphSize; /* glyph instructions buffer size */
     FT_Byte*           glyphIns;  /* glyph instructions buffer */
@@ -213,8 +217,11 @@ FT_BEGIN_HEADER
     TT_CodeRangeTable  codeRangeTable;  /* table of valid code ranges */
                                         /* useful for the debugger   */
 
-    FT_UShort          storeSize;  /* size of current storage */
-    FT_Long*           storage;    /* storage area            */
+    FT_UShort          storeSize;    /* size of current storage */
+    FT_Long*           storage;      /* storage area            */
+    FT_UShort          glyfStoreSize;
+    FT_Long*           glyfStorage;  /* storage working copy for glyph */
+    FT_Long*           origStorage;
 
     FT_F26Dot6         period;     /* values used for the */
     FT_F26Dot6         phase;      /* `SuperRounding'     */