From 834acad825d6c963f4a6a92a90420a6a8876c3c0 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Fri, 2 Aug 2019 05:35:18 +0100 Subject: [PATCH] fix << on signed int warnings << on a negative number is undefined behaviour in C, and will trigger fuzzer warnings. --- libvips/arithmetic/boolean.c | 34 ++++++++++++++++++++++++++++++++- libvips/colour/LabQ2Lab.c | 5 +++-- libvips/conversion/cast.c | 33 +++++++++++++++++++++++++++++--- libvips/conversion/tilecache.c | 2 +- libvips/convolution/convi.c | 3 ++- libvips/foreign/ppm.c | 4 ++-- libvips/foreign/tiff2vips.c | 2 +- libvips/foreign/vips2tiff.c | 2 +- libvips/foreign/webp2vips.c | 3 ++- libvips/include/vips/internal.h | 8 ++++++++ libvips/iofuncs/base64.c | 6 +++--- 11 files changed, 86 insertions(+), 16 deletions(-) diff --git a/libvips/arithmetic/boolean.c b/libvips/arithmetic/boolean.c index 1fc8aa3f..1df7dab3 100644 --- a/libvips/arithmetic/boolean.c +++ b/libvips/arithmetic/boolean.c @@ -73,6 +73,7 @@ #include #include +#include #include "binary.h" #include "unaryconst.h" @@ -140,6 +141,15 @@ vips_boolean_build( VipsObject *object ) g_assert_not_reached(); \ } +#define FNLOOP( TYPE, FN ) { \ + TYPE * restrict left = (TYPE *) in[0]; \ + TYPE * restrict right = (TYPE *) in[1]; \ + int * restrict q = (int *) out; \ + \ + for( x = 0; x < sz; x++ ) \ + q[x] = FN( left[x], right[x] ); \ +} + static void vips_boolean_buffer( VipsArithmetic *arithmetic, VipsPel *out, VipsPel **in, int width ) @@ -163,8 +173,30 @@ vips_boolean_buffer( VipsArithmetic *arithmetic, SWITCH( LOOP, FLOOP, ^ ); break; + /* Special case: we need to be able to use VIPS_LSHIFT_INT(). + */ case VIPS_OPERATION_BOOLEAN_LSHIFT: - SWITCH( LOOP, FLOOP, << ); + switch( vips_image_get_format( im ) ) { + case VIPS_FORMAT_UCHAR: + LOOP( unsigned char, << ); break; + case VIPS_FORMAT_CHAR: + FNLOOP( signed char, VIPS_LSHIFT_INT ); break; + case VIPS_FORMAT_USHORT: + LOOP( unsigned short, << ); break; + case VIPS_FORMAT_SHORT: + FNLOOP( signed short, VIPS_LSHIFT_INT ); break; + case VIPS_FORMAT_UINT: + LOOP( unsigned int, << ); break; + case VIPS_FORMAT_INT: + FNLOOP( signed int, VIPS_LSHIFT_INT ); break; + case VIPS_FORMAT_FLOAT: + FLOOP( float, << ); break; + case VIPS_FORMAT_DOUBLE: + FLOOP( double, << ); break; + + default: + g_assert_not_reached(); + } break; case VIPS_OPERATION_BOOLEAN_RSHIFT: diff --git a/libvips/colour/LabQ2Lab.c b/libvips/colour/LabQ2Lab.c index b29b0c49..0adc6b80 100644 --- a/libvips/colour/LabQ2Lab.c +++ b/libvips/colour/LabQ2Lab.c @@ -55,6 +55,7 @@ #include #include +#include #include "pcolour.h" @@ -95,12 +96,12 @@ vips_LabQ2Lab_line( VipsColour *colour, VipsPel *out, VipsPel **in, int width ) /* Build a. */ - l = (p[1] << 3) | ((lsbs >> 3) & 0x7); + l = VIPS_LSHIFT_INT( p[1], 3) | ((lsbs >> 3) & 0x7); q[1] = (float) l * 0.125; /* And b. */ - l = (p[2] << 3) | (lsbs & 0x7); + l = VIPS_LSHIFT_INT( p[2], 3) | (lsbs & 0x7); q[2] = (float) l * 0.125; p += 4; diff --git a/libvips/conversion/cast.c b/libvips/conversion/cast.c index 26fa7be4..1a7a8848 100644 --- a/libvips/conversion/cast.c +++ b/libvips/conversion/cast.c @@ -158,6 +158,18 @@ G_DEFINE_TYPE( VipsCast, vips_cast, VIPS_TYPE_CONVERSION ); q[x] = (p[x] << n) | (((p[x] & 1) << n) - (p[x] & 1)); \ } +#define SHIFT_LEFT_SIGNED( ITYPE, OTYPE ) { \ + ITYPE * restrict p = (ITYPE *) in; \ + OTYPE * restrict q = (OTYPE *) out; \ + int n = ((int) sizeof( OTYPE ) << 3) - ((int) sizeof( ITYPE ) << 3); \ + \ + g_assert( sizeof( ITYPE ) < sizeof( OTYPE ) ); \ + \ + for( x = 0; x < sz; x++ ) \ + q[x] = VIPS_LSHIFT_INT( p[x], n ) | \ + (((p[x] & 1) << n) - (p[x] & 1)); \ +} + /* Cast int types to an int type. We need to pass in the type of the * intermediate value, either uint or int, or we'll have problems with uint * sources turning -ve. @@ -188,6 +200,21 @@ G_DEFINE_TYPE( VipsCast, vips_cast, VIPS_TYPE_CONVERSION ); } \ } +/* Int to int handling for signed int types. + */ +#define INT_INT_SIGNED( ITYPE, OTYPE, TEMP, CAST ) { \ + if( cast->shift && \ + sizeof( ITYPE ) > sizeof( OTYPE ) ) { \ + SHIFT_RIGHT( ITYPE, OTYPE ); \ + } \ + else if( cast->shift ) { \ + SHIFT_LEFT_SIGNED( ITYPE, OTYPE ); \ + } \ + else { \ + CAST_INT_INT( ITYPE, OTYPE, TEMP, CAST ); \ + } \ +} + /* Cast float types to an int type. */ #define CAST_FLOAT_INT( ITYPE, OTYPE, TEMP, CAST ) { \ @@ -336,7 +363,7 @@ vips_cast_gen( VipsRegion *or, void *vseq, void *a, void *b, gboolean *stop ) case VIPS_FORMAT_CHAR: BAND_SWITCH_INNER( signed char, - INT_INT, + INT_INT_SIGNED, CAST_REAL_FLOAT, CAST_REAL_COMPLEX ); break; @@ -350,7 +377,7 @@ vips_cast_gen( VipsRegion *or, void *vseq, void *a, void *b, gboolean *stop ) case VIPS_FORMAT_SHORT: BAND_SWITCH_INNER( signed short, - INT_INT, + INT_INT_SIGNED, CAST_REAL_FLOAT, CAST_REAL_COMPLEX ); break; @@ -364,7 +391,7 @@ vips_cast_gen( VipsRegion *or, void *vseq, void *a, void *b, gboolean *stop ) case VIPS_FORMAT_INT: BAND_SWITCH_INNER( signed int, - INT_INT, + INT_INT_SIGNED, CAST_REAL_FLOAT, CAST_REAL_COMPLEX ); break; diff --git a/libvips/conversion/tilecache.c b/libvips/conversion/tilecache.c index 86e8dd30..dae7e440 100644 --- a/libvips/conversion/tilecache.c +++ b/libvips/conversion/tilecache.c @@ -418,7 +418,7 @@ vips_rect_hash( VipsRect *pos ) * X discrimination is more important than Y, since * most tiles will have a similar Y. */ - hash = pos->left ^ (pos->top << 16); + hash = pos->left ^ VIPS_LSHIFT_INT( pos->top, 16 ); return( hash ); } diff --git a/libvips/convolution/convi.c b/libvips/convolution/convi.c index 527c15e3..20aff567 100644 --- a/libvips/convolution/convi.c +++ b/libvips/convolution/convi.c @@ -123,6 +123,7 @@ #include #include +#include #include "pconvolution.h" @@ -979,7 +980,7 @@ vips_convi_intize( VipsConvi *convi, VipsImage *M ) if( convi->exp > 0 ) int_value = (int_sum + (1 << (convi->exp - 1))) >> convi->exp; else - int_value = int_sum << convi->exp; + int_value = VIPS_LSHIFT_INT( int_sum, convi->exp ); int_value = VIPS_CLIP( 0, int_value, 255 ); if( VIPS_ABS( true_value - int_value ) > 2 ) { diff --git a/libvips/foreign/ppm.c b/libvips/foreign/ppm.c index 7f9265d2..7fe27c01 100644 --- a/libvips/foreign/ppm.c +++ b/libvips/foreign/ppm.c @@ -406,7 +406,7 @@ read_1bit_binary( FILE *fp, VipsImage *out ) for( y = 0; y < out->Ysize; y++ ) { for( x = 0; x < out->Xsize * out->Bands; x++ ) { buf[x] = (bits & 128) ? 0 : 255; - bits <<= 1; + bits = VIPS_LSHIFT_INT( bits, 1 ); if( (x & 7) == 7 ) bits = fgetc( fp ); } @@ -667,7 +667,7 @@ write_ppm_line_binary_squash( Write *write, VipsPel *p ) bits = 0; n_bits = 0; for( x = 0; x < write->in->Xsize; x++ ) { - bits <<= 1; + bits = VIPS_LSHIFT_INT( bits, 1 ); n_bits += 1; bits |= p[x] ? 0 : 1; diff --git a/libvips/foreign/tiff2vips.c b/libvips/foreign/tiff2vips.c index 33106784..84792fc5 100644 --- a/libvips/foreign/tiff2vips.c +++ b/libvips/foreign/tiff2vips.c @@ -1066,7 +1066,7 @@ rtiff_palette_line_bit( Rtiff *rtiff, } } else - *q++ = i << (8 - bits_per_sample); + *q++ = VIPS_LSHIFT_INT( i, 8 - bits_per_sample ); } } diff --git a/libvips/foreign/vips2tiff.c b/libvips/foreign/vips2tiff.c index 782f0197..fca3d64e 100644 --- a/libvips/foreign/vips2tiff.c +++ b/libvips/foreign/vips2tiff.c @@ -1245,7 +1245,7 @@ LabS2Lab16( VipsPel *q, VipsPel *p, int n ) for( x = 0; x < n; x++ ) { /* TIFF uses unsigned 16 bit ... move zero, scale up L. */ - q1[0] = (int) p1[0] << 1; + q1[0] = VIPS_LSHIFT_INT( (int) p1[0], 1 ); q1[1] = p1[1]; q1[2] = p1[2]; diff --git a/libvips/foreign/webp2vips.c b/libvips/foreign/webp2vips.c index f5bc67a3..c12cdc46 100644 --- a/libvips/foreign/webp2vips.c +++ b/libvips/foreign/webp2vips.c @@ -248,7 +248,8 @@ vips_image_paint_area( VipsImage *image, const VipsRect *r, const VipsPel *ink ) /* Rebuild RGBA, assuming little-endian. */ -#define setRGBA( R, G, B, A ) (R | (G << 8) | (B << 16) | (A << 24)) +#define setRGBA( R, G, B, A ) \ + (R | (G << 8) | (B << 16) | ((guint32) A << 24)) /* OVER blend of two unpremultiplied RGBA guint32 * diff --git a/libvips/include/vips/internal.h b/libvips/include/vips/internal.h index 19fb57bd..3ecd3f1c 100644 --- a/libvips/include/vips/internal.h +++ b/libvips/include/vips/internal.h @@ -39,6 +39,14 @@ extern "C" { #endif /*__cplusplus*/ +/* << on an int is undefined in C if the int is negative. Imagine a machine + * that uses 1s complement, for example. + * + * Fuzzers find and warn about this, so we must use this macro instead. Cast + * to uint, shift, and cast back. + */ +#define VIPS_LSHIFT_INT( I, N ) ((int) ((unsigned int) (I) << (N))) + /* What we store in the Meta hash table. We can't just use GHashTable's * key/value pairs, since we need to iterate over meta in Meta_traverse order. * diff --git a/libvips/iofuncs/base64.c b/libvips/iofuncs/base64.c index fe9dc2cf..3e7b30c4 100644 --- a/libvips/iofuncs/base64.c +++ b/libvips/iofuncs/base64.c @@ -143,7 +143,7 @@ read24( const unsigned char *in, int remaining ) bits = 0; for( i = 0; i < 3; i++ ) { - bits <<= 8; + bits = VIPS_LSHIFT_INT( bits, 8 ); if( remaining > 0 ) { bits |= in[i]; remaining -= 1; @@ -167,7 +167,7 @@ encode24( char *p, int bits, int remaining ) /* Take the top 6 bits of 24. */ p[i] = b64_list[(bits >> 18) & 63]; - bits <<= 6; + bits = VIPS_LSHIFT_INT( bits, 6 ); remaining -= 6; } } @@ -281,7 +281,7 @@ vips__b64_decode( const char *buffer, size_t *data_length ) unsigned int val; if( (val = b64_index[(int) buffer[i]]) != XX ) { - bits <<= 6; + bits = VIPS_LSHIFT_INT( bits, 6 ); bits |= val; nbits += 6;