From fbe2fe4c7513246a666fb0cffffd29e7f6f3af4b Mon Sep 17 00:00:00 2001 From: Werner Lemberg Date: Mon, 29 May 2017 13:29:28 +0200 Subject: [PATCH] Handle some integer overflow run-time errors (#46149, #48979). This commit (mainly for 32bit CPUs) is the first of a series of similar commits to handle known integer overflows. Basically, all of them are harmless, since they affect rendering of glyphs only, not posing security threats. It is expected that fuzzying will show up more overflows, to be fixed in due course. The idea is to mark places where overflows can occur, using macros that simply cast to unsigned integers, because overflow arithmetic is well defined in this case. Doing so suppresses run-time errors of sanitizers without adding computational overhead. * include/freetype/internal/ftcalc.h (OVERFLOW_ADD_INT, OVERFLOW_SUB_INT, OVERFLOW_MUL_INT, OVERFLOW_ADD_LONG, OVERFLOW_SUB_LONG, OVERFLOW_MUL_LONG): New macros. * src/base/ftcalc.c (FT_RoundFix, FT_CeilFix, FT_Matrix_Multiply, FT_Matrix_Multiply_Scaled, FT_Vector_Transform_Scaled, ft_corner_orientation): Use new macros. * src/base/ftoutln.c (FT_Outline_Get_Orientation): Use new macros. --- ChangeLog | 25 +++++++++++++ include/freetype/freetype.h | 6 +++ include/freetype/ftglyph.h | 3 ++ include/freetype/internal/ftcalc.h | 23 ++++++++++++ src/base/ftcalc.c | 60 ++++++++++++++++++------------ src/base/ftoutln.c | 4 +- 6 files changed, 97 insertions(+), 24 deletions(-) diff --git a/ChangeLog b/ChangeLog index 5d7c7d82f..7a9244ef9 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,28 @@ +2017-05-29 Werner Lemberg + + Handle some integer overflow run-time errors (#46149, #48979). + + This commit (mainly for 32bit CPUs) is the first of a series of + similar commits to handle known integer overflows. Basically, all + of them are harmless, since they affect rendering of glyphs only, + not posing security threats. It is expected that fuzzying will show + up more overflows, to be fixed in due course. + + The idea is to mark places where overflows can occur, using macros + that simply cast to unsigned integers, because overflow arithmetic + is well defined in this case. Doing so suppresses run-time errors + of sanitizers without adding computational overhead. + + * include/freetype/internal/ftcalc.h (OVERFLOW_ADD_INT, + OVERFLOW_SUB_INT, OVERFLOW_MUL_INT, OVERFLOW_ADD_LONG, + OVERFLOW_SUB_LONG, OVERFLOW_MUL_LONG): New macros. + + * src/base/ftcalc.c (FT_RoundFix, FT_CeilFix, FT_Matrix_Multiply, + FT_Matrix_Multiply_Scaled, FT_Vector_Transform_Scaled, + ft_corner_orientation): Use new macros. + + * src/base/ftoutln.c (FT_Outline_Get_Orientation): Use new macros. + 2017-05-28 Werner Lemberg * include/freetype/internal/ftcalc.h (FLOAT_TO_FIXED): Remove. diff --git a/include/freetype/freetype.h b/include/freetype/freetype.h index 2989fbb5e..bd7f6a3fb 100644 --- a/include/freetype/freetype.h +++ b/include/freetype/freetype.h @@ -4327,6 +4327,9 @@ FT_BEGIN_HEADER /* `a' rounded to the nearest 16.16 fixed integer, halfway cases away */ /* from zero. */ /* */ + /* */ + /* The function uses wrap-around arithmetic. */ + /* */ FT_EXPORT( FT_Fixed ) FT_RoundFix( FT_Fixed a ); @@ -4345,6 +4348,9 @@ FT_BEGIN_HEADER /* */ /* `a' rounded towards plus infinity. */ /* */ + /* */ + /* The function uses wrap-around arithmetic. */ + /* */ FT_EXPORT( FT_Fixed ) FT_CeilFix( FT_Fixed a ); diff --git a/include/freetype/ftglyph.h b/include/freetype/ftglyph.h index 79879a7ac..5869bc1ce 100644 --- a/include/freetype/ftglyph.h +++ b/include/freetype/ftglyph.h @@ -566,6 +566,9 @@ FT_BEGIN_HEADER /* */ /* The result is undefined if either `a' or `b' is zero. */ /* */ + /* Since the function uses wrap-around arithmetic, results become */ + /* meaningless if the arguments are very large. */ + /* */ FT_EXPORT( void ) FT_Matrix_Multiply( const FT_Matrix* a, FT_Matrix* b ); diff --git a/include/freetype/internal/ftcalc.h b/include/freetype/internal/ftcalc.h index df6c3766d..1cd32c892 100644 --- a/include/freetype/internal/ftcalc.h +++ b/include/freetype/internal/ftcalc.h @@ -408,6 +408,29 @@ FT_BEGIN_HEADER #define ROUND_F26DOT6( x ) ( x >= 0 ? ( ( (x) + 32 ) & -64 ) \ : ( -( ( 32 - (x) ) & -64 ) ) ) + /* + * The following macros have two purposes. + * + * . Tag places where overflow is expected and harmless. + * + * . Avoid run-time sanitizer errors. + * + * Use with care! + */ +#define OVERFLOW_ADD_INT( a, b ) \ + (FT_Int)( (FT_UInt)(a) + (FT_UInt)(b) ) +#define OVERFLOW_SUB_INT( a, b ) \ + (FT_Int)( (FT_UInt)(a) - (FT_UInt)(b) ) +#define OVERFLOW_MUL_INT( a, b ) \ + (FT_Int)( (FT_UInt)(a) * (FT_UInt)(b) ) + +#define OVERFLOW_ADD_LONG( a, b ) \ + (FT_Long)( (FT_ULong)(a) + (FT_ULong)(b) ) +#define OVERFLOW_SUB_LONG( a, b ) \ + (FT_Long)( (FT_ULong)(a) - (FT_ULong)(b) ) +#define OVERFLOW_MUL_LONG( a, b ) \ + (FT_Long)( (FT_ULong)(a) * (FT_ULong)(b) ) + FT_END_HEADER diff --git a/src/base/ftcalc.c b/src/base/ftcalc.c index b4b66e402..bc1c47ff1 100644 --- a/src/base/ftcalc.c +++ b/src/base/ftcalc.c @@ -87,7 +87,8 @@ FT_EXPORT_DEF( FT_Fixed ) FT_RoundFix( FT_Fixed a ) { - return ( a + 0x8000L - ( a < 0 ) ) & ~0xFFFFL; + return ( OVERFLOW_ADD_LONG( a, + 0x8000L - ( a < 0 ) ) ) & ~0xFFFFL; } @@ -96,7 +97,7 @@ FT_EXPORT_DEF( FT_Fixed ) FT_CeilFix( FT_Fixed a ) { - return ( a + 0xFFFFL ) & ~0xFFFFL; + return ( OVERFLOW_ADD_LONG( a, 0xFFFFL ) ) & ~0xFFFFL; } @@ -667,13 +668,19 @@ if ( !a || !b ) return; - xx = FT_MulFix( a->xx, b->xx ) + FT_MulFix( a->xy, b->yx ); - xy = FT_MulFix( a->xx, b->xy ) + FT_MulFix( a->xy, b->yy ); - yx = FT_MulFix( a->yx, b->xx ) + FT_MulFix( a->yy, b->yx ); - yy = FT_MulFix( a->yx, b->xy ) + FT_MulFix( a->yy, b->yy ); + xx = OVERFLOW_ADD_LONG( FT_MulFix( a->xx, b->xx ), + FT_MulFix( a->xy, b->yx ) ); + xy = OVERFLOW_ADD_LONG( FT_MulFix( a->xx, b->xy ), + FT_MulFix( a->xy, b->yy ) ); + yx = OVERFLOW_ADD_LONG( FT_MulFix( a->yx, b->xx ), + FT_MulFix( a->yy, b->yx ) ); + yy = OVERFLOW_ADD_LONG( FT_MulFix( a->yx, b->xy ), + FT_MulFix( a->yy, b->yy ) ); - b->xx = xx; b->xy = xy; - b->yx = yx; b->yy = yy; + b->xx = xx; + b->xy = xy; + b->yx = yx; + b->yy = yy; } @@ -723,13 +730,19 @@ if ( !a || !b ) return; - xx = FT_MulDiv( a->xx, b->xx, val ) + FT_MulDiv( a->xy, b->yx, val ); - xy = FT_MulDiv( a->xx, b->xy, val ) + FT_MulDiv( a->xy, b->yy, val ); - yx = FT_MulDiv( a->yx, b->xx, val ) + FT_MulDiv( a->yy, b->yx, val ); - yy = FT_MulDiv( a->yx, b->xy, val ) + FT_MulDiv( a->yy, b->yy, val ); + xx = OVERFLOW_ADD_LONG( FT_MulDiv( a->xx, b->xx, val ), + FT_MulDiv( a->xy, b->yx, val ) ); + xy = OVERFLOW_ADD_LONG( FT_MulDiv( a->xx, b->xy, val ), + FT_MulDiv( a->xy, b->yy, val ) ); + yx = OVERFLOW_ADD_LONG( FT_MulDiv( a->yx, b->xx, val ), + FT_MulDiv( a->yy, b->yx, val ) ); + yy = OVERFLOW_ADD_LONG( FT_MulDiv( a->yx, b->xy, val ), + FT_MulDiv( a->yy, b->yy, val ) ); - b->xx = xx; b->xy = xy; - b->yx = yx; b->yy = yy; + b->xx = xx; + b->xy = xy; + b->yx = yx; + b->yy = yy; } @@ -748,11 +761,10 @@ if ( !vector || !matrix ) return; - xz = FT_MulDiv( vector->x, matrix->xx, val ) + - FT_MulDiv( vector->y, matrix->xy, val ); - - yz = FT_MulDiv( vector->x, matrix->yx, val ) + - FT_MulDiv( vector->y, matrix->yy, val ); + xz = OVERFLOW_ADD_LONG( FT_MulDiv( vector->x, matrix->xx, val ), + FT_MulDiv( vector->y, matrix->xy, val ) ); + yz = OVERFLOW_ADD_LONG( FT_MulDiv( vector->x, matrix->yx, val ), + FT_MulDiv( vector->y, matrix->yy, val ) ); vector->x = xz; vector->y = yz; @@ -914,11 +926,13 @@ FT_Int result; - if ( (FT_ULong)FT_ABS( in_x ) + (FT_ULong)FT_ABS( out_y ) <= 131071UL && - (FT_ULong)FT_ABS( in_y ) + (FT_ULong)FT_ABS( out_x ) <= 131071UL ) + /* we silently ignore overflow errors, since such large values */ + /* lead to even more (harmless) rendering errors later on */ + if ( OVERFLOW_ADD_LONG( FT_ABS( in_x ), FT_ABS( out_y ) ) <= 131071L && + OVERFLOW_ADD_LONG( FT_ABS( in_y ), FT_ABS( out_x ) ) <= 131071L ) { - FT_Long z1 = in_x * out_y; - FT_Long z2 = in_y * out_x; + FT_Long z1 = OVERFLOW_MUL_LONG( in_x, out_y ); + FT_Long z2 = OVERFLOW_MUL_LONG( in_y, out_x ); if ( z1 > z2 ) diff --git a/src/base/ftoutln.c b/src/base/ftoutln.c index 464a066dc..794ddbc47 100644 --- a/src/base/ftoutln.c +++ b/src/base/ftoutln.c @@ -1088,7 +1088,9 @@ v_cur.x = points[n].x >> xshift; v_cur.y = points[n].y >> yshift; - area += ( v_cur.y - v_prev.y ) * ( v_cur.x + v_prev.x ); + area = OVERFLOW_ADD_LONG( + area, + ( v_cur.y - v_prev.y ) * ( v_cur.x + v_prev.x ) ); v_prev = v_cur; }