Fix UBSan errors (#1948)

* Ensure UBSan exits with a non-zero code on error

* Avoid misaligned member access in mosaic_fuzzer

* Add missing VIPS_CLIP in scRGB2sRGB/scRGB2BW

* Fix UBSan error in flatten

By using saturated casts for the int types (copied from vips_cast).

* CI: ensure fuzzer log is printed on error

* Avoid UB in heifload

* Revert flatten change

I could no longer reproduce this with clang 12 locally.

* Indentation fixes [skip ci]

.github/workflows/ci.yml

@@ -110,7 +110,7 @@ jobs:
           echo "ASAN_OPTIONS=suppressions=${{ github.workspace }}/suppressions/asan.supp" >> $GITHUB_ENV
           echo "LSAN_OPTIONS=suppressions=${{ github.workspace }}/suppressions/lsan.supp" >> $GITHUB_ENV
           echo "TSAN_OPTIONS=suppressions=${{ github.workspace }}/suppressions/tsan.supp" >> $GITHUB_ENV
-          echo "UBSAN_OPTIONS=suppressions=${{ github.workspace }}/suppressions/ubsan.supp:print_stacktrace=1" >> $GITHUB_ENV
+          echo "UBSAN_OPTIONS=suppressions=${{ github.workspace }}/suppressions/ubsan.supp:halt_on_error=1:abort_on_error=1" >> $GITHUB_ENV
           echo "LD_LIBRARY_PATH=$LLVM_PREFIX/lib:`dirname $ASAN_DSO`" >> $GITHUB_ENV
           echo "$LLVM_PREFIX/bin" >> $GITHUB_PATH
           # workaround for https://github.com/google/sanitizers/issues/89
@@ -128,7 +128,7 @@ jobs:
         run: make V=0 -j$JOBS
 
       - name: Check libvips
-        run: make V=0 check || (cat test/test-suite.log && exit 1)
+        run: make V=0 VERBOSE=1 check
 
       - name: Install libvips
         run: sudo make V=0 install

fuzz/mosaic_fuzzer.cc

@@ -6,7 +6,7 @@ struct mosaic_opt {
 	guint16 yref;
 	guint16 xsec;
 	guint16 ysec;
-};
+} __attribute__ ((packed));
 
 extern "C" int
 LLVMFuzzerInitialize( int *argc, char ***argv )
@@ -19,16 +19,23 @@ extern "C" int
 LLVMFuzzerTestOneInput( const guint8 *data, size_t size )
 {
 	VipsImage *ref, *sec, *out;
-	struct mosaic_opt *opt;
+	mosaic_opt opt = {};
 	double d;
 
-	if( size < sizeof( struct mosaic_opt ) )
+	if( size < sizeof( mosaic_opt ) )
 		return( 0 );
 
 	if( size > 100 * 1024 * 1024 )
 		return( 0 );
 
-	if( !(ref = vips_image_new_from_buffer( data, size, "", NULL )) )
+	/* The beginning of `data` is treated as mosaic configuration
+	 */
+	memcpy( &opt, data, sizeof( mosaic_opt ) );
+
+	/* Remainder of input is the image
+	 */
+	if( !(ref = vips_image_new_from_buffer( data + sizeof( mosaic_opt ),
+		size - sizeof( mosaic_opt ), "", NULL )) )
 		return( 0 );
 
 	if( ref->Xsize > 100 ||
@@ -43,12 +50,8 @@ LLVMFuzzerTestOneInput( const guint8 *data, size_t size )
 		return( 0 );
 	}
 
-	/* Extract some bytes from the tail to fuzz the arguments of the API.
-	 */
-	opt = (struct mosaic_opt *) (data + size - sizeof( struct mosaic_opt ));
-
-	if( vips_mosaic( ref, sec, &out, (VipsDirection) opt->dir,
-		opt->xref, opt->yref, opt->xsec, opt->ysec, NULL ) ) {
+	if( vips_mosaic( ref, sec, &out, (VipsDirection) opt.dir,
+		opt.xref, opt.yref, opt.xsec, opt.ysec, NULL ) ) {
 		g_object_unref( sec );
 		g_object_unref( ref );
 		return( 0 );

fuzz/test_fuzz.sh

@@ -7,8 +7,8 @@ set -e
 
 # Glib is built without -fno-omit-frame-pointer. We need
 # to disable the fast unwinder to get full stacktraces.
-export ASAN_OPTIONS="fast_unwind_on_malloc=0:allocator_may_return_null=1"
-export UBSAN_OPTIONS="print_stacktrace=1"
+export ASAN_OPTIONS="$ASAN_OPTIONS:fast_unwind_on_malloc=0:allocator_may_return_null=1"
+export UBSAN_OPTIONS="$UBSAN_OPTIONS:print_stacktrace=1"
 
 # Hide all warning messages from vips.
 export VIPS_WARNING=0
@@ -17,7 +17,9 @@ ret=0
 
 for fuzzer in *_fuzzer; do
   for file in $top_srcdir/fuzz/common_fuzzer_corpus/*; do
-    if ! ./$fuzzer $file; then
+    exit_code=0
+    ./$fuzzer $file || exit_code=$?
+    if [ $exit_code -ne 0 ]; then
       echo FAIL $fuzzer $file
       ret=1
     fi

libvips/colour/scRGB2BW.c

@@ -84,7 +84,7 @@ vips_scRGB2BW_line_8( VipsPel * restrict q, float * restrict p,
 		q += 1;
 
 		for( j = 0; j < extra_bands; j++ ) 
-			q[j] = p[j];
+			q[j] = VIPS_CLIP( 0, p[j], UCHAR_MAX );
 		p += extra_bands;
 		q += extra_bands;
 	}

libvips/colour/scRGB2sRGB.c

@@ -110,7 +110,7 @@ vips_scRGB2sRGB_line_8( VipsPel * restrict q, float * restrict p,
 		q += 3;
 
 		for( j = 0; j < extra_bands; j++ ) 
-			q[j] = p[j];
+			q[j] = VIPS_CLIP( 0, p[j], UCHAR_MAX );
 		p += extra_bands;
 		q += extra_bands;
 	}

libvips/conversion/flatten.c

@@ -83,7 +83,7 @@ G_DEFINE_TYPE( VipsFlatten, vips_flatten, VIPS_TYPE_CONVERSION );
 
 /* Flatten with black background.
  */
-#define VIPS_FLATTEN_BLACK( TYPE ) { \
+#define VIPS_FLATTEN_BLACK_INT( TYPE ) { \
 	TYPE * restrict p = (TYPE *) in; \
 	TYPE * restrict q = (TYPE *) out; \
 	\
@@ -120,7 +120,7 @@ G_DEFINE_TYPE( VipsFlatten, vips_flatten, VIPS_TYPE_CONVERSION );
 
 /* Flatten with any background.
  */
-#define VIPS_FLATTEN( TYPE ) { \
+#define VIPS_FLATTEN_INT( TYPE ) { \
 	TYPE * restrict p = (TYPE *) in; \
 	TYPE * restrict q = (TYPE *) out; \
 	\
@@ -179,11 +179,11 @@ vips_flatten_black_gen( VipsRegion *or, void *vseq, void *a, void *b,
 
 		switch( flatten->in->BandFmt ) { 
 		case VIPS_FORMAT_UCHAR: 
-			VIPS_FLATTEN_BLACK( unsigned char ); 
+			VIPS_FLATTEN_BLACK_INT( unsigned char ); 
 			break; 
 
 		case VIPS_FORMAT_CHAR: 
-			VIPS_FLATTEN_BLACK( signed char ); 
+			VIPS_FLATTEN_BLACK_INT( signed char ); 
 			break; 
 
 		case VIPS_FORMAT_USHORT: 
@@ -244,11 +244,11 @@ vips_flatten_gen( VipsRegion *or, void *vseq, void *a, void *b,
 
 		switch( flatten->in->BandFmt ) { 
 		case VIPS_FORMAT_UCHAR: 
-			VIPS_FLATTEN( unsigned char ); 
+			VIPS_FLATTEN_INT( unsigned char ); 
 			break; 
 
 		case VIPS_FORMAT_CHAR: 
-			VIPS_FLATTEN( signed char ); 
+			VIPS_FLATTEN_INT( signed char ); 
 			break; 
 
 		case VIPS_FORMAT_USHORT: 

libvips/foreign/heifload.c

@@ -279,7 +279,11 @@ static int
 vips_foreign_load_heif_is_a( const char *buf, int len )
 {
 	if( len >= 12 ) {
-		const guint chunk_len = GUINT_FROM_BE( *((guint32 *) buf) );
+		const guint32 chunk_len = 
+			(guint32) buf[0] << 24 |
+			(guint32) buf[1] << 16 |
+			(guint32) buf[2] << 8 |
+			(guint32) buf[3];
 
 		int i;