1 files changed, 58 insertions, 32 deletions

diff --git a/freetype/src/cff/cffgload.c b/freetype/src/cff/cffgload.c
index a988d5117..6dce4f3a1 100644
--- a/freetype/src/cff/cffgload.c
+++ b/freetype/src/cff/cffgload.c
@@ -2280,6 +2280,8 @@
           /* subsequent `pop' operands should add the arguments,       */
           /* this is the implementation described for `unknown' other  */
           /* subroutines in the Type1 spec.                            */
+          /*                                                           */
+          /* XXX Fix return arguments (see discussion below).          */
           args -= 2 + ( args[-2] >> 16 );
           if ( args < stack )
             goto Stack_Underflow;
@@ -2292,6 +2294,22 @@
 
           FT_TRACE4(( " pop (invalid op)\n" ));
 
+          /* XXX Increasing `args' is wrong: After a certain number of */
+          /* `pop's we get a stack overflow.  Reason for doing it is   */
+          /* code like this (actually found in a CFF font):            */
+          /*                                                           */
+          /*   17 1 3 callothersubr                                    */
+          /*   pop                                                     */
+          /*   callsubr                                                */
+          /*                                                           */
+          /* Since we handle `callothersubr' as a no-op, and           */
+          /* `callsubr' needs at least one argument, `pop' can't be a  */
+          /* no-op too as it basically should be.                      */
+          /*                                                           */
+          /* The right solution would be to provide real support for   */
+          /* `callothersubr' as done in `t1decode.c', however, given   */
+          /* the fact that CFF fonts with `pop' are invalid, it is     */
+          /* questionable whether it is worth the time.                */
           args++;
           break;
 
@@ -2455,7 +2473,10 @@
           return CFF_Err_Unimplemented_Feature;
         }
 
-      decoder->top = args;
+        decoder->top = args;
+
+        if ( decoder->top - stack >= CFF_MAX_OPERANDS )
+          goto Stack_Overflow;
 
       } /* general operator processing */
 
@@ -2728,48 +2749,53 @@
       /* now load the unscaled outline */
       error = cff_get_glyph_data( face, glyph_index,
                                   &charstring, &charstring_len );
-      if ( !error )
-      {
-        error = cff_decoder_prepare( &decoder, size, glyph_index );
-        if ( !error )
-        {
-          error = cff_decoder_parse_charstrings( &decoder,
-                                                 charstring,
-                                                 charstring_len );
+      if ( error )
+        goto Glyph_Build_Finished;
 
-          cff_free_glyph_data( face, &charstring, charstring_len );
+      error = cff_decoder_prepare( &decoder, size, glyph_index );
+      if ( error )
+        goto Glyph_Build_Finished;
+
+      error = cff_decoder_parse_charstrings( &decoder,
+                                             charstring,
+                                             charstring_len );
 
+      cff_free_glyph_data( face, &charstring, charstring_len );
+
+      if ( error )
+        goto Glyph_Build_Finished;
 
 #ifdef FT_CONFIG_OPTION_INCREMENTAL
-          /* Control data and length may not be available for incremental */
-          /* fonts.                                                       */
-          if ( face->root.internal->incremental_interface )
-          {
-            glyph->root.control_data = 0;
-            glyph->root.control_len = 0;
-          }
-          else
+      /* Control data and length may not be available for incremental */
+      /* fonts.                                                       */
+      if ( face->root.internal->incremental_interface )
+      {
+        glyph->root.control_data = 0;
+        glyph->root.control_len = 0;
+      }
+      else
 #endif /* FT_CONFIG_OPTION_INCREMENTAL */
 
-          /* We set control_data and control_len if charstrings is loaded. */
-          /* See how charstring loads at cff_index_access_element() in     */
-          /* cffload.c.                                                    */
-          {
-            CFF_Index  csindex = &cff->charstrings_index;
+      /* We set control_data and control_len if charstrings is loaded. */
+      /* See how charstring loads at cff_index_access_element() in     */
+      /* cffload.c.                                                    */
+      {
+        CFF_Index  csindex = &cff->charstrings_index;
 
 
-            if ( csindex->offsets )
-            {
-              glyph->root.control_data = csindex->bytes +
-                                           csindex->offsets[glyph_index] - 1;
-              glyph->root.control_len  = charstring_len;
-            }
-          }
+        if ( csindex->offsets )
+        {
+          glyph->root.control_data = csindex->bytes +
+                                     csindex->offsets[glyph_index] - 1;
+          glyph->root.control_len  = charstring_len;
         }
       }
 
-      /* save new glyph tables */
-      cff_builder_done( &decoder.builder );
+  Glyph_Build_Finished:
+      /* save new glyph tables, if no error */
+      if ( !error )
+        cff_builder_done( &decoder.builder );
+      /* XXX: anything to do for broken glyph entry? */
     }
 
 #ifdef FT_CONFIG_OPTION_INCREMENTAL