Introduce unlimited flag, enabling unlimited text chunks in PNGs (#2419)

* Introduce unlimited flag, enabling unlimited text chunks

* remove extraneous optional flag causing GLib-GObject-CRITICAL **: 17:10:34.095: validate_and_install_class_property

* Address feedback from @jcupitt

* various compilation error fixes

* Interlace detection uses unlimited=false

* attempt to fix tests

* documentation changes, introduced MAX_PNG_TEXT_CHUNKS and bumped max to 20 (was 10)

cplusplus/include/vips/VImage8.h

@@ -4562,6 +4562,7 @@ VImage phasecor( VImage in2, VOption *options = 0 ) const;
  *   - **sequential** -- Sequential read only, bool.
  *   - **fail** -- Fail on first error, bool.
  *   - **disc** -- Open to disc, bool.
+ *   - **unlimited** -- Remove all denial of service limits.
  *
  * @param filename Filename to load from.
  * @param options Set of options.
@@ -4578,6 +4579,7 @@ static VImage pngload( const char *filename, VOption *options = 0 );
  *   - **sequential** -- Sequential read only, bool.
  *   - **fail** -- Fail on first error, bool.
  *   - **disc** -- Open to disc, bool.
+ *   - **unlimited** -- Remove all denial of service limits.
  *
  * @param buffer Buffer to load from.
  * @param options Set of options.
@@ -4594,6 +4596,7 @@ static VImage pngload_buffer( VipsBlob *buffer, VOption *options = 0 );
  *   - **sequential** -- Sequential read only, bool.
  *   - **fail** -- Fail on first error, bool.
  *   - **disc** -- Open to disc, bool.
+ *   - **unlimited** -- Remove all denial of service limits.
  *
  * @param source Source to load from.
  * @param options Set of options.

libvips/foreign/pforeign.h

@@ -45,6 +45,8 @@ extern "C" {
 )
 #endif /*HAVE_CHECKED_MUL*/
 
+#define MAX_PNG_TEXT_CHUNKS 20
+
 void vips__tiff_init( void );
 
 int vips__tiff_write( VipsImage *in, const char *filename, 
@@ -177,9 +179,9 @@ int vips__jpeg_read_source( VipsSource *source, VipsImage *out,
 int vips__isjpeg_source( VipsSource *source );
 
 int vips__png_ispng_source( VipsSource *source );
-int vips__png_header_source( VipsSource *source, VipsImage *out );
+int vips__png_header_source( VipsSource *source, VipsImage *out, gboolean unlimited );
 int vips__png_read_source( VipsSource *source, VipsImage *out, 
-	gboolean fail );
+	gboolean fail, gboolean unlimited );
 gboolean vips__png_isinterlaced_source( VipsSource *source );
 extern const char *vips__png_suffs[];
 

libvips/foreign/pngload.c

@@ -59,6 +59,10 @@ typedef struct _VipsForeignLoadPng {
 	 */
 	VipsSource *source;
 
+	/* remove all denial of service limits.
+	 */
+	gboolean unlimited;
+
 } VipsForeignLoadPng;
 
 typedef VipsForeignLoadClass VipsForeignLoadPngClass;
@@ -118,7 +122,7 @@ vips_foreign_load_png_header( VipsForeignLoad *load )
 {
 	VipsForeignLoadPng *png = (VipsForeignLoadPng *) load;
 
-	if( vips__png_header_source( png->source, load->out ) )
+	if( vips__png_header_source( png->source, load->out, png->unlimited ) )
 		return( -1 );
 
 	return( 0 );
@@ -129,7 +133,7 @@ vips_foreign_load_png_load( VipsForeignLoad *load )
 {
 	VipsForeignLoadPng *png = (VipsForeignLoadPng *) load;
 
-	if( vips__png_read_source( png->source, load->real, load->fail ) )
+	if( vips__png_read_source( png->source, load->real, load->fail, png->unlimited ) )
 		return( -1 );
 
 	return( 0 );
@@ -144,6 +148,8 @@ vips_foreign_load_png_class_init( VipsForeignLoadPngClass *class )
 	VipsForeignLoadClass *load_class = (VipsForeignLoadClass *) class;
 
 	gobject_class->dispose = vips_foreign_load_png_dispose;
+	gobject_class->set_property = vips_object_set_property;
+	gobject_class->get_property = vips_object_get_property;
 
 	object_class->nickname = "pngload_base";
 	object_class->description = _( "load png base class" );
@@ -158,6 +164,12 @@ vips_foreign_load_png_class_init( VipsForeignLoadPngClass *class )
 	load_class->header = vips_foreign_load_png_header;
 	load_class->load = vips_foreign_load_png_load;
 
+	VIPS_ARG_BOOL( class, "unlimited", 23,
+		_( "Unlimited" ),
+		_( "Remove all denial of service limits" ),
+		VIPS_ARGUMENT_OPTIONAL_INPUT,
+		G_STRUCT_OFFSET( VipsForeignLoadPng, unlimited ),
+		FALSE );
 }
 
 static void
@@ -395,12 +407,20 @@ vips_foreign_load_png_buffer_init( VipsForeignLoadPngBuffer *buffer )
  * @out: (out): decompressed image
  * @...: %NULL-terminated list of optional named arguments
  *
+ * Optional arguments:
+ *
+ * * @unlimited: %gboolean, remove all denial of service limits
+ *
  * Read a PNG file into a VIPS image. It can read all png images, including 8-
  * and 16-bit images, 1 and 3 channel, with and without an alpha channel.
  *
  * Any ICC profile is read and attached to the VIPS image. It also supports
  * XMP metadata.
  *
+ * By default, the PNG loader limits the number of text and data chunks to 
+ * block some denial of service attacks. Set @unlimited to disable these 
+ * limits.
+ *
  * See also: vips_image_new_from_file().
  *
  * Returns: 0 on success, -1 on error.
@@ -425,6 +445,10 @@ vips_pngload( const char *filename, VipsImage **out, ... )
  * @out: (out): image to write
  * @...: %NULL-terminated list of optional named arguments
  *
+ * Optional arguments:
+ *
+ * * @unlimited: %gboolean, Remove all denial of service limits
+ *
  * Exactly as vips_pngload(), but read from a PNG-formatted memory block.
  *
  * You must not free the buffer while @out is active. The 
@@ -460,6 +484,10 @@ vips_pngload_buffer( void *buf, size_t len, VipsImage **out, ... )
  * @out: (out): image to write
  * @...: %NULL-terminated list of optional named arguments
  *
+ * Optional arguments:
+ *
+ * * @unlimited: %gboolean, Remove all denial of service limits
+ *
  * Exactly as vips_pngload(), but read from a source. 
  *
  * See also: vips_pngload().

libvips/foreign/spngload.c

@@ -63,6 +63,10 @@ typedef struct _VipsForeignLoadPng {
 	 */
 	VipsSource *source;
 
+	/* Remove DoS limits.
+	 */
+	gboolean unlimited;
+
 	spng_ctx *ctx;
 	struct spng_ihdr ihdr;
 	enum spng_format fmt;
@@ -249,7 +253,8 @@ vips_foreign_load_png_set_header( VipsForeignLoadPng *png, VipsImage *image )
 		/* Very large numbers of text chunks are used in DoS
 		 * attacks.
 		 */
-		if( n_text > 10 ) {
+
+		if( !png->unlimited && n_text > MAX_PNG_TEXT_CHUNKS ) {
 			vips_error( class->nickname, 
 				"%s", _( "too many text chunks" ) );
 			return( -1 );
@@ -349,8 +354,10 @@ vips_foreign_load_png_header( VipsForeignLoad *load )
 	 * No need to test the decoded image size -- the user can do that if
 	 * they wish.
 	 */
-	spng_set_image_limits( png->ctx, VIPS_MAX_COORD, VIPS_MAX_COORD );
+	if ( !png->unlimited ) {
-	spng_set_chunk_limits( png->ctx, 60 * 1024 * 1024, 60 * 1024 * 1024 );
+		spng_set_image_limits( png->ctx, VIPS_MAX_COORD, VIPS_MAX_COORD );
+		spng_set_chunk_limits( png->ctx, 60 * 1024 * 1024, 60 * 1024 * 1024 );
+	}
 
 	if( vips_source_rewind( png->source ) ) 
 		return( -1 );
@@ -648,6 +655,8 @@ vips_foreign_load_png_class_init( VipsForeignLoadPngClass *class )
 	VipsForeignLoadClass *load_class = (VipsForeignLoadClass *) class;
 
 	gobject_class->dispose = vips_foreign_load_png_dispose;
+	gobject_class->set_property = vips_object_set_property;
+	gobject_class->get_property = vips_object_get_property;
 
 	object_class->nickname = "pngload_base";
 	object_class->description = _( "load png base class" );
@@ -662,6 +671,12 @@ vips_foreign_load_png_class_init( VipsForeignLoadPngClass *class )
 	load_class->header = vips_foreign_load_png_header;
 	load_class->load = vips_foreign_load_png_load;
 
+	VIPS_ARG_BOOL( class, "unlimited", 23,
+		_( "Unlimited" ),
+		_( "Remove all denial of service limits" ),
+		VIPS_ARGUMENT_OPTIONAL_INPUT,
+		G_STRUCT_OFFSET( VipsForeignLoadPng, unlimited ),
+		FALSE );
 }
 
 static void

libvips/foreign/vipspng.c

@@ -173,6 +173,7 @@ typedef struct {
 	char *name;
 	VipsImage *out;
 	gboolean fail;
+	gboolean unlimited;
 
 	int y_pos;
 	png_structp pPng;
@@ -255,7 +256,7 @@ vips_png_read_source( png_structp pPng, png_bytep data, png_size_t length )
 }
 
 static Read *
-read_new( VipsSource *source, VipsImage *out, gboolean fail )
+read_new( VipsSource *source, VipsImage *out, gboolean fail, gboolean unlimited )
 {
 	Read *read;
 
@@ -270,6 +271,8 @@ read_new( VipsSource *source, VipsImage *out, gboolean fail )
 	read->pInfo = NULL;
 	read->row_pointer = NULL;
 	read->source = source;
+	read->unlimited = unlimited;
+
 	g_object_ref( source );
 
 	g_signal_connect( out, "close", 
@@ -564,7 +567,7 @@ png2vips_header( Read *read, VipsImage *out )
 		/* Very large numbers of text chunks are used in DoS
 		 * attacks.
 		 */
-		if( num_text > 10 ) {
+		if( !read->unlimited && num_text > MAX_PNG_TEXT_CHUNKS ) {
 			vips_error( "vipspng", 
 				"%s", _( "too many text chunks" ) );
 			return( -1 );
@@ -778,11 +781,11 @@ vips__png_ispng_source( VipsSource *source )
 }
 
 int
-vips__png_header_source( VipsSource *source, VipsImage *out )
+vips__png_header_source( VipsSource *source, VipsImage *out, gboolean unlimited )
 {
 	Read *read;
 
-	if( !(read = read_new( source, out, TRUE )) ||
+	if( !(read = read_new( source, out, TRUE, unlimited )) ||
 		png2vips_header( read, out ) ) {
 		vips_error( "png2vips", _( "unable to read source %s" ),
 			vips_connection_nick( VIPS_CONNECTION( source ) ) );
@@ -795,11 +798,11 @@ vips__png_header_source( VipsSource *source, VipsImage *out )
 }
 
 int
-vips__png_read_source( VipsSource *source, VipsImage *out, gboolean fail )
+vips__png_read_source( VipsSource *source, VipsImage *out, gboolean fail, gboolean unlimited )
 {
 	Read *read;
 
-	if( !(read = read_new( source, out, fail )) ||
+	if( !(read = read_new( source, out, fail, unlimited )) ||
 		png2vips_image( read, out ) ||
 		vips_source_decode( source ) ) {
 		vips_error( "png2vips", _( "unable to read source %s" ),
@@ -822,7 +825,7 @@ vips__png_isinterlaced_source( VipsSource *source )
 
 	image = vips_image_new();
 
-	if( !(read = read_new( source, image, TRUE )) ) { 
+	if( !(read = read_new( source, image, TRUE, FALSE )) ) { 
 		g_object_unref( image );
 		return( -1 );
 	}