freetype-commit
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[freetype2] master ded4bdb 3/4: [base] More sanity checks for Mac resour


From: Werner LEMBERG
Subject: [freetype2] master ded4bdb 3/4: [base] More sanity checks for Mac resources.
Date: Wed, 21 Dec 2016 05:53:00 +0000 (UTC)

branch: master
commit ded4bdb5d0fce69fe0ac804b15c695a71b31d3ad
Author: Werner Lemberg <address@hidden>
Commit: Werner Lemberg <address@hidden>

    [base] More sanity checks for Mac resources.
    
    We use
    
      https://github.com/kreativekorp/ksfl/wiki/Macintosh-Resource-File-Format
    
    and
    
      
https://developer.apple.com/legacy/library/documentation/mac/pdf/MoreMacintoshToolbox.pdf#page=151
    
    as references.
    
    * include/freetype/internal/ftrfork.h (FT_RFork_Ref): Use FT_Short
    for `res_id'.
    
    * src/base/ftrfork.c (FT_Raccess_Get_HeaderInfo): Extract map length
    and use it to improve sanity checks.
    Follow the specification more closely;in particular, all data types
    are signed, not unsigned.
    (FT_Raccess_Get_DataOffsets): Follow the specification more closely;
    in particular, all data types are signed, not unsigned.
    Add some sanity checks.
---
 ChangeLog                           |   25 ++++++++
 include/freetype/internal/ftrfork.h |    5 +-
 src/base/ftrfork.c                  |  107 ++++++++++++++++++++++++++---------
 3 files changed, 109 insertions(+), 28 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 3719683..8ecd242 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,30 @@
 2016-12-20  Werner Lemberg  <address@hidden>
 
+       [base] More sanity checks for Mac resources.
+
+       We use
+
+         
https://github.com/kreativekorp/ksfl/wiki/Macintosh-Resource-File-Format
+
+       and
+
+         
https://developer.apple.com/legacy/library/documentation/mac/pdf/MoreMacintoshToolbox.pdf#page=151
+
+       as references.
+
+       * include/freetype/internal/ftrfork.h (FT_RFork_Ref): Use FT_Short
+       for `res_id'.
+
+       * src/base/ftrfork.c (FT_Raccess_Get_HeaderInfo): Extract map length
+       and use it to improve sanity checks.
+       Follow the specification more closely;in particular, all data types
+       are signed, not unsigned.
+       (FT_Raccess_Get_DataOffsets): Follow the specification more closely;
+       in particular, all data types are signed, not unsigned.
+       Add some sanity checks.
+
+2016-12-20  Werner Lemberg  <address@hidden>
+
        [truetype] Improve logic for getting fast advance widths.
 
        * src/cff/cffdrivr.c (cff_get_advances), src/truetype/ttdriver.c
diff --git a/include/freetype/internal/ftrfork.h 
b/include/freetype/internal/ftrfork.h
index b923401..718fa62 100644
--- a/include/freetype/internal/ftrfork.h
+++ b/include/freetype/internal/ftrfork.h
@@ -43,11 +43,12 @@ FT_BEGIN_HEADER
 
   typedef struct  FT_RFork_Ref_
   {
-    FT_UShort  res_id;
-    FT_Long    offset;
+    FT_Short  res_id;
+    FT_Long   offset;
 
   } FT_RFork_Ref;
 
+
 #ifdef FT_CONFIG_OPTION_GUESSING_EMBEDDED_RFORK
   typedef FT_Error
   (*ft_raccess_guess_func)( FT_Library  library,
diff --git a/src/base/ftrfork.c b/src/base/ftrfork.c
index 4660c97..b8b97a8 100644
--- a/src/base/ftrfork.c
+++ b/src/base/ftrfork.c
@@ -56,7 +56,7 @@
   {
     FT_Error       error;
     unsigned char  head[16], head2[16];
-    FT_Long        map_pos, rdata_len;
+    FT_Long        map_pos, map_len, rdata_len;
     int            allzeros, allmatch, i;
     FT_Long        type_list;
 
@@ -67,12 +67,15 @@
     if ( error )
       return error;
 
-    error = FT_Stream_Read( stream, (FT_Byte *)head, 16 );
+    error = FT_Stream_Read( stream, (FT_Byte*)head, 16 );
     if ( error )
       return error;
 
     /* ensure positive values */
-    if ( head[0] >= 0x80 || head[4] >= 0x80 || head[8] >= 0x80 )
+    if ( head[0]  >= 0x80 ||
+         head[4]  >= 0x80 ||
+         head[8]  >= 0x80 ||
+         head[12] >= 0x80 )
       return FT_THROW( Unknown_File_Format );
 
     *rdata_pos = ( head[ 0] << 24 ) |
@@ -87,14 +90,36 @@
                  ( head[ 9] << 16 ) |
                  ( head[10] <<  8 ) |
                    head[11];
+    map_len    = ( head[12] << 24 ) |
+                 ( head[13] << 16 ) |
+                 ( head[14] <<  8 ) |
+                   head[15];
 
-    /* map_len = head[12] .. head[15] */
-
-    if ( *rdata_pos != map_pos - rdata_len || map_pos == 0 )
+    /* the map must not be empty */
+    if ( !map_pos )
       return FT_THROW( Unknown_File_Format );
 
-    if ( FT_LONG_MAX - rfork_offset < *rdata_pos ||
-         FT_LONG_MAX - rfork_offset < map_pos    )
+    /* check whether rdata and map overlap */
+    if ( *rdata_pos < map_pos )
+    {
+      if ( *rdata_pos > map_pos - rdata_len )
+        return FT_THROW( Unknown_File_Format );
+    }
+    else
+    {
+      if ( map_pos > *rdata_pos - map_len )
+        return FT_THROW( Unknown_File_Format );
+    }
+
+    /* check whether end of rdata or map exceeds stream size */
+    if ( FT_LONG_MAX - rdata_len < *rdata_pos                               ||
+         FT_LONG_MAX - map_len < map_pos                                    ||
+
+         FT_LONG_MAX - ( *rdata_pos + rdata_len ) < rfork_offset            ||
+         FT_LONG_MAX - ( map_pos + map_len ) < rfork_offset                 ||
+
+         (FT_ULong)( rfork_offset + *rdata_pos + rdata_len ) > stream->size ||
+         (FT_ULong)( rfork_offset + map_pos + map_len ) > stream->size      )
       return FT_THROW( Unknown_File_Format );
 
     *rdata_pos += rfork_offset;
@@ -124,15 +149,14 @@
 
     /* If we have reached this point then it is probably a mac resource */
     /* file.  Now, does it contain any interesting resources?           */
-    /* Skip handle to next resource map, the file resource number, and  */
-    /* attributes.                                                      */
+
     (void)FT_STREAM_SKIP( 4        /* skip handle to next resource map */
                           + 2      /* skip file resource number */
                           + 2 );   /* skip attributes */
 
-    if ( FT_READ_USHORT( type_list ) )
+    if ( FT_READ_SHORT( type_list ) )
       return error;
-    if ( type_list == -1 )
+    if ( type_list < 0 )
       return FT_THROW( Unknown_File_Format );
 
     error = FT_Stream_Seek( stream, (FT_ULong)( map_pos + type_list ) );
@@ -181,15 +205,34 @@
     if ( error )
       return error;
 
-    if ( FT_READ_USHORT( cnt ) )
+    if ( FT_READ_SHORT( cnt ) )
       return error;
     cnt++;
 
+    /* `rpos' is a signed 16bit integer offset to resource records; the    */
+    /* size of a resource record is 12 bytes.  The map header is 28 bytes, */
+    /* and a type list needs 10 bytes or more.  If we assume that the name */
+    /* list is empty and we have only a single entry in the type list,     */
+    /* there can be at most                                                */
+    /*                                                                     */
+    /*   (32768 - 28 - 10) / 12 = 2727                                     */
+    /*                                                                     */
+    /* resources.                                                          */
+    /*                                                                     */
+    /* A type list starts with a two-byte counter, followed by 10-byte     */
+    /* type records.  Assuming that there are no resources, the number of  */
+    /* type records can be at most                                         */
+    /*                                                                     */
+    /*   (32768 - 28 - 2) / 8 = 4079                                       */
+    /*                                                                     */
+    if ( cnt > 4079 )
+      return FT_THROW( Invalid_Table );
+
     for ( i = 0; i < cnt; ++i )
     {
       if ( FT_READ_LONG( tag_internal ) ||
-           FT_READ_USHORT( subcnt )     ||
-           FT_READ_USHORT( rpos )       )
+           FT_READ_SHORT( subcnt )      ||
+           FT_READ_SHORT( rpos )        )
         return error;
 
       FT_TRACE2(( "Resource tags: %c%c%c%c\n",
@@ -205,6 +248,9 @@
         *count = subcnt + 1;
         rpos  += map_offset;
 
+        if ( *count > 2727 )
+          return FT_THROW( Invalid_Table );
+
         error = FT_Stream_Seek( stream, (FT_ULong)rpos );
         if ( error )
           return error;
@@ -214,33 +260,42 @@
 
         for ( j = 0; j < *count; ++j )
         {
-          if ( FT_READ_USHORT( ref[j].res_id ) )
+          if ( FT_READ_SHORT( ref[j].res_id ) )
             goto Exit;
-          if ( FT_STREAM_SKIP( 2 ) ) /* resource name */
+          if ( FT_STREAM_SKIP( 2 ) )  /* resource name offset */
             goto Exit;
-          if ( FT_READ_LONG( temp ) )
+          if ( FT_READ_LONG( temp ) ) /* attributes (8bit), offset (24bit) */
             goto Exit;
-          if ( FT_STREAM_SKIP( 4 ) ) /* mbz */
+          if ( FT_STREAM_SKIP( 4 ) )  /* mbz */
             goto Exit;
 
+          if ( ref[j].res_id < 0 || temp < 0 )
+          {
+            error = FT_THROW( Invalid_Table );
+            goto Exit;
+          }
+
           ref[j].offset = temp & 0xFFFFFFL;
+
           FT_TRACE3(( "             [%d]:"
                       " resource_id=0x%04x, offset=0x%08x\n",
                       j, ref[j].res_id, ref[j].offset ));
         }
 
-        if (sort_by_res_id)
+        if ( sort_by_res_id )
         {
-          ft_qsort( ref, (size_t)*count, sizeof ( FT_RFork_Ref ),
-                    ( int(*)(const void*, const void*) )
-                    ft_raccess_sort_ref_by_id );
+          ft_qsort( ref,
+                    (size_t)*count,
+                    sizeof ( FT_RFork_Ref ),
+                    ( int(*)(const void*,
+                             const void*) )ft_raccess_sort_ref_by_id );
 
           FT_TRACE3(( "             -- sort resources by their ids --\n" ));
-          for ( j = 0; j < *count; ++ j ) {
+
+          for ( j = 0; j < *count; j++ )
             FT_TRACE3(( "             [%d]:"
                         " resource_id=0x%04x, offset=0x%08x\n",
                         j, ref[j].res_id, ref[j].offset ));
-          }
         }
 
         if ( FT_NEW_ARRAY( offsets_internal, *count ) )
@@ -250,7 +305,7 @@
          *      gap between reference IDs are acceptable?
          *      further investigation on Apple implementation is needed.
          */
-        for ( j = 0; j < *count; ++j )
+        for ( j = 0; j < *count; j++ )
           offsets_internal[j] = rdata_pos + ref[j].offset;
 
         *offsets = offsets_internal;



reply via email to

[Prev in Thread] Current Thread [Next in Thread]