Commit 6343ba22a36ab264fa061cab3b87662d5b7b7114

Werner Lemberg 2015-08-01T07:53:48

Fix some bugs found by clang's `-fsanitize=undefined' (#45661). * src/base/ftrfork.c (FT_Raccess_Get_HeaderInfo): Only accept positive values from header. Check overflow. * src/base/ftoutln.c (SCALED): Correctly handle left-shift of negative values. * src/bdf/bdf.h (_bdf_glyph_modified, _bdf_set_glyph_modified, _bdf_clear_glyph_modified): Use unsigned long constant. * src/bdf/bdfdrivr.c (BDF_Size_Select, BDF_Glyph_Load): Don't left-shift values that can be negative. * src/pcf/pcfdrivr.c (PCF_Size_Select, PCF_Glyph_Load): Don't left-shift values that can be negative. * src/raster/ftraster.c (SCALED): Correctly handle left-shift of negative values. * src/sfnt/ttsbit.c (tt_face_load_strike_metrics): Don't left-shift values that can be negative. * src/truetype/ttgload.c (TT_Load_Composite_Glyph, compute_glyph_metrics, load_sbit_image): Don't left-shift values that can be negative.

diff --git a/ChangeLog b/ChangeLog
index a5e353b..efb8dd0 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,35 @@
 2015-07-31  Werner Lemberg  <wl@gnu.org>
 
+	Fix some bugs found by clang's `-fsanitize=undefined' (#45661).
+
+	* src/base/ftrfork.c (FT_Raccess_Get_HeaderInfo): Only accept
+	positive values from header.
+	Check overflow.
+
+	* src/base/ftoutln.c (SCALED): Correctly handle left-shift of
+	negative values.
+
+	* src/bdf/bdf.h (_bdf_glyph_modified, _bdf_set_glyph_modified,
+	_bdf_clear_glyph_modified): Use unsigned long constant.
+
+	* src/bdf/bdfdrivr.c (BDF_Size_Select, BDF_Glyph_Load): Don't
+	left-shift values that can be negative.
+
+	* src/pcf/pcfdrivr.c (PCF_Size_Select, PCF_Glyph_Load): Don't
+	left-shift values that can be negative.
+
+	* src/raster/ftraster.c (SCALED): Correctly handle left-shift of
+	negative values.
+
+	* src/sfnt/ttsbit.c (tt_face_load_strike_metrics): Don't left-shift
+	values that can be negative.
+
+	* src/truetype/ttgload.c (TT_Load_Composite_Glyph,
+	compute_glyph_metrics, load_sbit_image): Don't left-shift values
+	that can be negative.
+
+2015-07-31  Werner Lemberg  <wl@gnu.org>
+
 	Define FT_LONG_MAX.
 
 	* include/freetype/config/ftstdlib.h (FT_LONG_MAX): New macro.
diff --git a/src/base/ftoutln.c b/src/base/ftoutln.c
index f71a1e7..ce4bd6c 100644
--- a/src/base/ftoutln.c
+++ b/src/base/ftoutln.c
@@ -52,8 +52,9 @@
                         const FT_Outline_Funcs*  func_interface,
                         void*                    user )
   {
-#undef SCALED
-#define SCALED( x )  ( ( (x) << shift ) - delta )
+#undef  SCALED
+#define SCALED( x )  ( ( (x) < 0 ? -( -(x) << shift )             \
+                                 :  (  (x) << shift ) ) - delta )
 
     FT_Vector   v_last;
     FT_Vector   v_control;
diff --git a/src/base/ftrfork.c b/src/base/ftrfork.c
index 82d54f8..702f639 100644
--- a/src/base/ftrfork.c
+++ b/src/base/ftrfork.c
@@ -71,24 +71,35 @@
     if ( error )
       return error;
 
-    *rdata_pos = rfork_offset + ( ( head[0] << 24 ) |
-                                  ( head[1] << 16 ) |
-                                  ( head[2] <<  8 ) |
-                                    head[3]         );
-    map_pos    = rfork_offset + ( ( head[4] << 24 ) |
-                                  ( head[5] << 16 ) |
-                                  ( head[6] <<  8 ) |
-                                    head[7]         );
-    rdata_len = ( head[ 8] << 24 ) |
-                ( head[ 9] << 16 ) |
-                ( head[10] <<  8 ) |
-                  head[11];
+    /* ensure positive values */
+    if ( head[0] >= 0x80 || head[4] >= 0x80 || head[8] >= 0x80 )
+      return FT_THROW( Unknown_File_Format );
+
+    *rdata_pos = ( head[ 0] << 24 ) |
+                 ( head[ 1] << 16 ) |
+                 ( head[ 2] <<  8 ) |
+                   head[ 3];
+    map_pos    = ( head[ 4] << 24 ) |
+                 ( head[ 5] << 16 ) |
+                 ( head[ 6] <<  8 ) |
+                   head[ 7];
+    rdata_len  = ( head[ 8] << 24 ) |
+                 ( head[ 9] << 16 ) |
+                 ( head[10] <<  8 ) |
+                   head[11];
 
     /* map_len = head[12] .. head[15] */
 
-    if ( *rdata_pos + rdata_len != map_pos || map_pos == rfork_offset )
+    if ( *rdata_pos != map_pos - rdata_len || map_pos == 0 )
       return FT_THROW( Unknown_File_Format );
 
+    if ( FT_LONG_MAX - rfork_offset > *rdata_pos ||
+         FT_LONG_MAX - rfork_offset > map_pos    )
+      return FT_THROW( Unknown_File_Format );
+
+    *rdata_pos += rfork_offset;
+    map_pos    += rfork_offset;
+
     error = FT_Stream_Seek( stream, (FT_ULong)map_pos );
     if ( error )
       return error;
diff --git a/src/bdf/bdf.h b/src/bdf/bdf.h
index bd5a2e4..f24d925 100644
--- a/src/bdf/bdf.h
+++ b/src/bdf/bdf.h
@@ -40,12 +40,12 @@ FT_BEGIN_HEADER
 
 /* Imported from bdfP.h */
 
-#define _bdf_glyph_modified( map, e )                 \
-          ( (map)[(e) >> 5] & ( 1 << ( (e) & 31 ) ) )
-#define _bdf_set_glyph_modified( map, e )              \
-          ( (map)[(e) >> 5] |= ( 1 << ( (e) & 31 ) ) )
-#define _bdf_clear_glyph_modified( map, e )             \
-          ( (map)[(e) >> 5] &= ~( 1 << ( (e) & 31 ) ) )
+#define _bdf_glyph_modified( map, e )                     \
+          ( (map)[(e) >> 5] & ( 1UL << ( (e) & 31 ) ) )
+#define _bdf_set_glyph_modified( map, e )                 \
+          ( (map)[(e) >> 5] |= ( 1UL << ( (e) & 31 ) ) )
+#define _bdf_clear_glyph_modified( map, e )               \
+          ( (map)[(e) >> 5] &= ~( 1UL << ( (e) & 31 ) ) )
 
 /* end of bdfP.h */
 
diff --git a/src/bdf/bdfdrivr.c b/src/bdf/bdfdrivr.c
index 4b3fb76..8bd9eeb 100644
--- a/src/bdf/bdfdrivr.c
+++ b/src/bdf/bdfdrivr.c
@@ -615,9 +615,9 @@ THE SOFTWARE.
 
     FT_Select_Metrics( size->face, strike_index );
 
-    size->metrics.ascender    = bdffont->font_ascent << 6;
-    size->metrics.descender   = -bdffont->font_descent << 6;
-    size->metrics.max_advance = bdffont->bbx.width << 6;
+    size->metrics.ascender    = bdffont->font_ascent * 64;
+    size->metrics.descender   = -bdffont->font_descent * 64;
+    size->metrics.max_advance = bdffont->bbx.width * 64;
 
     return FT_Err_Ok;
   }
@@ -734,18 +734,18 @@ THE SOFTWARE.
     slot->bitmap_left = glyph.bbx.x_offset;
     slot->bitmap_top  = glyph.bbx.ascent;
 
-    slot->metrics.horiAdvance  = (FT_Pos)( glyph.dwidth << 6 );
-    slot->metrics.horiBearingX = (FT_Pos)( glyph.bbx.x_offset << 6 );
-    slot->metrics.horiBearingY = (FT_Pos)( glyph.bbx.ascent << 6 );
-    slot->metrics.width        = (FT_Pos)( bitmap->width << 6 );
-    slot->metrics.height       = (FT_Pos)( bitmap->rows << 6 );
+    slot->metrics.horiAdvance  = (FT_Pos)( glyph.dwidth * 64 );
+    slot->metrics.horiBearingX = (FT_Pos)( glyph.bbx.x_offset * 64 );
+    slot->metrics.horiBearingY = (FT_Pos)( glyph.bbx.ascent * 64 );
+    slot->metrics.width        = (FT_Pos)( bitmap->width * 64 );
+    slot->metrics.height       = (FT_Pos)( bitmap->rows * 64 );
 
     /*
      * XXX DWIDTH1 and VVECTOR should be parsed and
      * used here, provided such fonts do exist.
      */
     ft_synthesize_vertical_metrics( &slot->metrics,
-                                    bdf->bdffont->bbx.height << 6 );
+                                    bdf->bdffont->bbx.height * 64 );
 
   Exit:
     return error;
diff --git a/src/pcf/pcfdrivr.c b/src/pcf/pcfdrivr.c
index 552049e..5984adc 100644
--- a/src/pcf/pcfdrivr.c
+++ b/src/pcf/pcfdrivr.c
@@ -430,9 +430,9 @@ THE SOFTWARE.
 
     FT_Select_Metrics( size->face, strike_index );
 
-    size->metrics.ascender    =  accel->fontAscent << 6;
-    size->metrics.descender   = -accel->fontDescent << 6;
-    size->metrics.max_advance =  accel->maxbounds.characterWidth << 6;
+    size->metrics.ascender    =  accel->fontAscent * 64;
+    size->metrics.descender   = -accel->fontDescent * 64;
+    size->metrics.max_advance =  accel->maxbounds.characterWidth * 64;
 
     return FT_Err_Ok;
   }
@@ -583,16 +583,16 @@ THE SOFTWARE.
     slot->bitmap_left = metric->leftSideBearing;
     slot->bitmap_top  = metric->ascent;
 
-    slot->metrics.horiAdvance  = (FT_Pos)( metric->characterWidth << 6 );
-    slot->metrics.horiBearingX = (FT_Pos)( metric->leftSideBearing << 6 );
-    slot->metrics.horiBearingY = (FT_Pos)( metric->ascent << 6 );
+    slot->metrics.horiAdvance  = (FT_Pos)( metric->characterWidth * 64 );
+    slot->metrics.horiBearingX = (FT_Pos)( metric->leftSideBearing * 64 );
+    slot->metrics.horiBearingY = (FT_Pos)( metric->ascent * 64 );
     slot->metrics.width        = (FT_Pos)( ( metric->rightSideBearing -
-                                             metric->leftSideBearing ) << 6 );
-    slot->metrics.height       = (FT_Pos)( bitmap->rows << 6 );
+                                             metric->leftSideBearing ) * 64 );
+    slot->metrics.height       = (FT_Pos)( bitmap->rows * 64 );
 
     ft_synthesize_vertical_metrics( &slot->metrics,
                                     ( face->accel.fontAscent +
-                                      face->accel.fontDescent ) << 6 );
+                                      face->accel.fontDescent ) * 64 );
 
   Exit:
     return error;
diff --git a/src/raster/ftraster.c b/src/raster/ftraster.c
index 373f5d2..8e9c794 100644
--- a/src/raster/ftraster.c
+++ b/src/raster/ftraster.c
@@ -450,7 +450,9 @@
 #define CEILING( x )  ( ( (x) + ras.precision - 1 ) & -ras.precision )
 #define TRUNC( x )    ( (Long)(x) >> ras.precision_bits )
 #define FRAC( x )     ( (x) & ( ras.precision - 1 ) )
-#define SCALED( x )   ( ( (Long)(x) << ras.scale_shift ) - ras.precision_half )
+#define SCALED( x )   ( ( (x) < 0 ? -( -(x) << ras.scale_shift )   \
+                                  :  (  (x) << ras.scale_shift ) ) \
+                        - ras.precision_half )
 
 #define IS_BOTTOM_OVERSHOOT( x ) \
           (Bool)( CEILING( x ) - x >= ras.precision_half )
diff --git a/src/sfnt/ttsbit.c b/src/sfnt/ttsbit.c
index 143f276..3b351ec 100644
--- a/src/sfnt/ttsbit.c
+++ b/src/sfnt/ttsbit.c
@@ -254,15 +254,15 @@
         metrics->x_ppem = (FT_UShort)strike[44];
         metrics->y_ppem = (FT_UShort)strike[45];
 
-        metrics->ascender  = (FT_Char)strike[16] << 6;  /* hori.ascender  */
-        metrics->descender = (FT_Char)strike[17] << 6;  /* hori.descender */
+        metrics->ascender  = (FT_Char)strike[16] * 64;  /* hori.ascender  */
+        metrics->descender = (FT_Char)strike[17] * 64;  /* hori.descender */
         metrics->height    = metrics->ascender - metrics->descender;
 
         /* Is this correct? */
         metrics->max_advance = ( (FT_Char)strike[22] + /* min_origin_SB  */
                                           strike[18] + /* max_width      */
                                  (FT_Char)strike[23]   /* min_advance_SB */
-                                                     ) << 6;
+                                                     ) * 64;
         return FT_Err_Ok;
       }
 
diff --git a/src/truetype/ttgload.c b/src/truetype/ttgload.c
index c35d835..979b74b 100644
--- a/src/truetype/ttgload.c
+++ b/src/truetype/ttgload.c
@@ -656,20 +656,20 @@
 
       if ( subglyph->flags & WE_HAVE_A_SCALE )
       {
-        xx = (FT_Fixed)FT_NEXT_SHORT( p ) << 2;
+        xx = (FT_Fixed)FT_NEXT_SHORT( p ) * 4;
         yy = xx;
       }
       else if ( subglyph->flags & WE_HAVE_AN_XY_SCALE )
       {
-        xx = (FT_Fixed)FT_NEXT_SHORT( p ) << 2;
-        yy = (FT_Fixed)FT_NEXT_SHORT( p ) << 2;
+        xx = (FT_Fixed)FT_NEXT_SHORT( p ) * 4;
+        yy = (FT_Fixed)FT_NEXT_SHORT( p ) * 4;
       }
       else if ( subglyph->flags & WE_HAVE_A_2X2 )
       {
-        xx = (FT_Fixed)FT_NEXT_SHORT( p ) << 2;
-        yx = (FT_Fixed)FT_NEXT_SHORT( p ) << 2;
-        xy = (FT_Fixed)FT_NEXT_SHORT( p ) << 2;
-        yy = (FT_Fixed)FT_NEXT_SHORT( p ) << 2;
+        xx = (FT_Fixed)FT_NEXT_SHORT( p ) * 4;
+        yx = (FT_Fixed)FT_NEXT_SHORT( p ) * 4;
+        xy = (FT_Fixed)FT_NEXT_SHORT( p ) * 4;
+        yy = (FT_Fixed)FT_NEXT_SHORT( p ) * 4;
       }
 
       subglyph->transform.xx = xx;
@@ -1988,7 +1988,7 @@
              ( ( ignore_x_mode && loader->exec->compatible_widths ) ||
                 !ignore_x_mode                                      ||
                 SPH_OPTION_BITMAP_WIDTHS                            ) )
-          glyph->metrics.horiAdvance = *widthp << 6;
+          glyph->metrics.horiAdvance = *widthp * 64;
       }
       else
 
@@ -1996,7 +1996,7 @@
 
       {
         if ( widthp )
-          glyph->metrics.horiAdvance = *widthp << 6;
+          glyph->metrics.horiAdvance = *widthp * 64;
       }
     }
 
@@ -2136,16 +2136,16 @@
       glyph->outline.n_points   = 0;
       glyph->outline.n_contours = 0;
 
-      glyph->metrics.width  = (FT_Pos)metrics.width  << 6;
-      glyph->metrics.height = (FT_Pos)metrics.height << 6;
+      glyph->metrics.width  = (FT_Pos)metrics.width  * 64;
+      glyph->metrics.height = (FT_Pos)metrics.height * 64;
 
-      glyph->metrics.horiBearingX = (FT_Pos)metrics.horiBearingX << 6;
-      glyph->metrics.horiBearingY = (FT_Pos)metrics.horiBearingY << 6;
-      glyph->metrics.horiAdvance  = (FT_Pos)metrics.horiAdvance  << 6;
+      glyph->metrics.horiBearingX = (FT_Pos)metrics.horiBearingX * 64;
+      glyph->metrics.horiBearingY = (FT_Pos)metrics.horiBearingY * 64;
+      glyph->metrics.horiAdvance  = (FT_Pos)metrics.horiAdvance  * 64;
 
-      glyph->metrics.vertBearingX = (FT_Pos)metrics.vertBearingX << 6;
-      glyph->metrics.vertBearingY = (FT_Pos)metrics.vertBearingY << 6;
-      glyph->metrics.vertAdvance  = (FT_Pos)metrics.vertAdvance  << 6;
+      glyph->metrics.vertBearingX = (FT_Pos)metrics.vertBearingX * 64;
+      glyph->metrics.vertBearingY = (FT_Pos)metrics.vertBearingY * 64;
+      glyph->metrics.vertAdvance  = (FT_Pos)metrics.vertAdvance  * 64;
 
       glyph->format = FT_GLYPH_FORMAT_BITMAP;