From 6343ba22a36ab264fa061cab3b87662d5b7b7114 Mon Sep 17 00:00:00 2001 From: Werner Lemberg Date: Sat, 1 Aug 2015 07:53:48 +0200 Subject: [PATCH] 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. --- ChangeLog | 30 ++++++++++++++++++++++++++++++ src/base/ftoutln.c | 5 +++-- src/base/ftrfork.c | 37 ++++++++++++++++++++++++------------- src/bdf/bdf.h | 12 ++++++------ src/bdf/bdfdrivr.c | 18 +++++++++--------- src/pcf/pcfdrivr.c | 18 +++++++++--------- src/raster/ftraster.c | 4 +++- src/sfnt/ttsbit.c | 6 +++--- src/truetype/ttgload.c | 34 +++++++++++++++++----------------- 9 files changed, 104 insertions(+), 60 deletions(-) diff --git a/ChangeLog b/ChangeLog index a5e353b83..efb8dd08f 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,33 @@ +2015-07-31 Werner Lemberg + + 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 Define FT_LONG_MAX. diff --git a/src/base/ftoutln.c b/src/base/ftoutln.c index f71a1e7c2..ce4bd6cd2 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 82d54f830..702f639af 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 bd5a2e433..f24d925d8 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 4b3fb7664..8bd9eeb8f 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 552049e9b..5984adc7b 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 373f5d2f8..8e9c79426 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 143f276d3..3b351ecfc 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 c35d835fe..979b74bc1 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;