Date: Tue, 10 Jun 2008 19:39:21 -0400
From: Josh Bressers <bressers@...hat.com>
To: oss-security@...ts.openwall.com
Subject: Re: FreeType 2.3.6

On 10 June 2008, Josh Bressers wrote:
> So it seems FreeType 2.3.6 fixes some security issues:
> 
>     - A  bunch of  potential security  problems have  been found.  All
>       users should update.
> 
> Does anyone have a freetype contact who we can try to convince to work with
> the community in the future (or give us patches for these)?
> 

After looking at the iDefense advisory, and through the Freetype changelog,
it looks like this is the changeset we need to fix this:

2008-06-08  Werner Lemberg

        * src/type1/t1parse.h (T1_ParserRec): Make `base_len' and
        `private_len' unsigned.

        * src/type1/t1parse.c (read_pfb_tag): Make `asize' unsigned and
        * read
        it as such.
        (T1_New_Parser, T1_Get_Private_Dict): Make `size' unsigned.


        * src/base/ftstream.c (FT_Stream_Skip): Reject negative values.


        * src/type1/t1load.c (parse_blend_design_positions): Check `n_axis'
        for sane value.
        Fix typo.


        * src/psaux/psobjs.c (ps_table_add): Check `idx' correctly.


        * src/truetype/ttinterp (Ins_SHC): Use BOUNDS() to check
        `last_point'.


        * src/sfnt/ttload.c (tt_face_load_max_profile): Limit
        `maxTwilightPoints'.

I'll attach the patch (with comment changes stripped).

-- 
    JB


diff --git a/src/base/ftstream.c b/src/base/ftstream.c
index a067a1f..569e46c 100644
--- a/src/base/ftstream.c
+++ b/src/base/ftstream.c
@@ -89,6 +89,9 @@
   FT_Stream_Skip( FT_Stream  stream,
                   FT_Long    distance )
   {
+    if ( distance < 0 )
+      return FT_Err_Invalid_Stream_Operation;
+
     return FT_Stream_Seek( stream, (FT_ULong)( stream->pos + distance ) );
   }
 
diff --git a/src/psaux/psobjs.c b/src/psaux/psobjs.c
index 9d3ebdf..b7b84ac 100644
--- a/src/psaux/psobjs.c
+++ b/src/psaux/psobjs.c
@@ -169,7 +169,7 @@
                 void*       object,
                 FT_PtrDist  length )
   {
-    if ( idx < 0 || idx > table->max_elems )
+    if ( idx < 0 || idx >= table->max_elems )
     {
       FT_ERROR(( "ps_table_add: invalid index\n" ));
       return PSaux_Err_Invalid_Argument;
diff --git a/src/sfnt/ttload.c b/src/sfnt/ttload.c
index abe0278..6b7c342 100644
--- a/src/sfnt/ttload.c
+++ b/src/sfnt/ttload.c
@@ -618,6 +618,15 @@
 
       if ( maxProfile->maxFunctionDefs == 0 )
         maxProfile->maxFunctionDefs = 64;
+
+      /* we add 4 phantom points later */
+      if ( maxProfile->maxTwilightPoints > ( 0xFFFFU - 4 ) )
+      {
+        FT_ERROR(( "Too much twilight points in `maxp' table;\n" ));
+        FT_ERROR(( "  some glyphs might be rendered incorrectly.\n" ));
+
+        maxProfile->maxTwilightPoints = 0xFFFFU - 4;
+      }
     }
 
     FT_TRACE3(( "numGlyphs: %u\n", maxProfile->numGlyphs ));
diff --git a/src/truetype/ttinterp.c b/src/truetype/ttinterp.c
index f0f91e9..f9c3656 100644
--- a/src/truetype/ttinterp.c
+++ b/src/truetype/ttinterp.c
@@ -5449,7 +5449,7 @@
 
     /* XXX: this is probably wrong... at least it prevents memory */
     /*      corruption when zp2 is the twilight zone              */
-    if ( last_point > CUR.zp2.n_points )
+    if ( BOUNDS( last_point, CUR.zp2.n_points ) )
     {
       if ( CUR.zp2.n_points > 0 )
         last_point = (FT_UShort)(CUR.zp2.n_points - 1);
diff --git a/src/type1/t1load.c b/src/type1/t1load.c
index 508fd89..9d7c748 100644
--- a/src/type1/t1load.c
+++ b/src/type1/t1load.c
@@ -674,7 +674,7 @@
 
       for ( n = 0; n < num_designs; n++ )
       {
-        T1_TokenRec  axis_tokens[T1_MAX_MM_DESIGNS];
+        T1_TokenRec  axis_tokens[T1_MAX_MM_AXIS];
         T1_Token     token;
         FT_Int       axis, n_axis;
 
@@ -687,6 +687,15 @@
 
         if ( n == 0 )
         {
+          if ( n_axis <= 0 || n_axis > T1_MAX_MM_AXIS )
+          {
+            FT_ERROR(( "parse_blend_design_positions:" ));
+            FT_ERROR(( "  invalid number of axes: %d\n",
+                       n_axis ));
+            error = T1_Err_Invalid_File_Format;
+            goto Exit;
+          }
+
           num_axis = n_axis;
           error = t1_allocate_blend( face, num_designs, num_axis );
           if ( error )
diff --git a/src/type1/t1parse.c b/src/type1/t1parse.c
index 1b252c7..36f5c82 100644
--- a/src/type1/t1parse.c
+++ b/src/type1/t1parse.c
@@ -65,14 +65,16 @@
   /*************************************************************************/
 
 
+  /* see Adobe Technical Note 5040.Download_Fonts.pdf */
+
   static FT_Error
   read_pfb_tag( FT_Stream   stream,
                 FT_UShort  *atag,
-                FT_Long    *asize )
+                FT_ULong   *asize )
   {
     FT_Error   error;
     FT_UShort  tag;
-    FT_Long    size;
+    FT_ULong   size;
 
 
     *atag  = 0;
@@ -82,7 +84,7 @@
     {
       if ( tag == 0x8001U || tag == 0x8002U )
       {
-        if ( !FT_READ_LONG_LE( size ) )
+        if ( !FT_READ_ULONG_LE( size ) )
           *asize = size;
       }
 
@@ -100,22 +102,25 @@
   {
     FT_Error   error;
     FT_UShort  tag;
-    FT_Long    size;
+    FT_ULong   dummy;
 
 
     if ( FT_STREAM_SEEK( 0 ) )
       goto Exit;
 
-    error = read_pfb_tag( stream, &tag, &size );
+    error = read_pfb_tag( stream, &tag, &dummy );
     if ( error )
       goto Exit;
 
+    /* We assume that the first segment in a PFB is always encoded as   */
+    /* text.  This might be wrong (and the specification doesn't insist */
+    /* on that), but we have never seen a counterexample.               */
     if ( tag != 0x8001U && FT_STREAM_SEEK( 0 ) )
       goto Exit;
 
     if ( !FT_FRAME_ENTER( header_length ) )
     {
-      error = 0;
+      error = T1_Err_Ok;
 
       if ( ft_memcmp( stream->cursor, header_string, header_length ) != 0 )
         error = T1_Err_Unknown_File_Format;
@@ -136,7 +141,7 @@
   {
     FT_Error   error;
     FT_UShort  tag;
-    FT_Long    size;
+    FT_ULong   size;
 
 
     psaux->ps_parser_funcs->init( &parser->root, 0, 0, memory );
@@ -260,7 +265,7 @@
     FT_Stream  stream = parser->stream;
     FT_Memory  memory = parser->root.memory;
     FT_Error   error  = T1_Err_Ok;
-    FT_Long    size;
+    FT_ULong   size;
 
 
     if ( parser->in_pfb )
@@ -299,7 +304,7 @@
         goto Fail;
       }
 
-      if ( FT_STREAM_SEEK( start_pos )                             ||
+      if ( FT_STREAM_SEEK( start_pos )                           ||
            FT_ALLOC( parser->private_dict, parser->private_len ) )
         goto Fail;
 
@@ -409,7 +414,7 @@
         goto Exit;
       }
 
-      size = (FT_Long)( parser->base_len - ( cur - parser->base_dict ) );
+      size = parser->base_len - ( cur - parser->base_dict );
 
       if ( parser->in_memory )
       {
diff --git a/src/type1/t1parse.h b/src/type1/t1parse.h
index 6fa4ca6..fb1c8a8 100644
--- a/src/type1/t1parse.h
+++ b/src/type1/t1parse.h
@@ -64,10 +64,10 @@ FT_BEGIN_HEADER
     FT_Stream     stream;
 
     FT_Byte*      base_dict;
-    FT_Long       base_len;
+    FT_ULong      base_len;
 
     FT_Byte*      private_dict;
-    FT_Long       private_len;
+    FT_ULong      private_len;
 
     FT_Bool       in_pfb;
     FT_Bool       in_memory;

Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.

Hosted by DataForce ISP - Powered by Openwall GNU/*/Linux