From 78295188474682070b685aec0fd7c8bcdd3b4099 Mon Sep 17 00:00:00 2001 From: Lovell Fuller Date: Fri, 25 Sep 2020 18:51:48 +0100 Subject: [PATCH 1/2] heifload: prevent reading beyond end of source buffer --- libvips/foreign/heifload.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/libvips/foreign/heifload.c b/libvips/foreign/heifload.c index 7ee306f6..f9c3830c 100644 --- a/libvips/foreign/heifload.c +++ b/libvips/foreign/heifload.c @@ -961,7 +961,11 @@ vips_foreign_load_heif_wait_for_file_size( gint64 target_size, void *userdata ) enum heif_reader_grow_status status; - if( heif->length == -1 ) + if( heif->source->data != NULL && target_size > heif->source->length ) + /* Target size is beyond known buffer length + */ + status = heif_reader_grow_status_size_beyond_eof; + else if( heif->length == -1 ) /* We've not seen EOF yet, so seeking to any point is fine (as * far as we know). */ From 237604bb078bd971608c0b93fe893d2882c8cf18 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Sat, 26 Sep 2020 13:00:52 +0100 Subject: [PATCH 2/2] revise heifload EOF detection VipsSource used a unix-style model where read() returns 0 to mean EOF. libheif uses a model where a separate call to wait_for_file_size() beforehand is used to check thaht the read will be OK, and then the read() is expected to never fail. We were trying to put EOF detection into libheif read(), but that's not the right way to do it. Instead, test for EOF in wait_for_file_size(). see https://github.com/libvips/libvips/pull/1833 --- ChangeLog | 1 + libvips/foreign/heifload.c | 49 ++++++++++++++++---------------------- 2 files changed, 22 insertions(+), 28 deletions(-) diff --git a/ChangeLog b/ChangeLog index 1f75af7f..b2225875 100644 --- a/ChangeLog +++ b/ChangeLog @@ -9,6 +9,7 @@ - allow gaussblur sigma zero, meaning no blur - better heif signature detection [lovell] - fix vips_fractsurf() typo [kleisauke] +- better heif EOF detection [lovell] 9/8/20 started 8.10.1 - fix markdown -> xml conversion in doc generation diff --git a/libvips/foreign/heifload.c b/libvips/foreign/heifload.c index f9c3830c..02122f73 100644 --- a/libvips/foreign/heifload.c +++ b/libvips/foreign/heifload.c @@ -173,10 +173,6 @@ typedef struct _VipsForeignLoadHeif { */ struct heif_reader *reader; - /* When we see EOF from read(), record the source length here. - */ - gint64 length; - } VipsForeignLoadHeif; typedef struct _VipsForeignLoadHeifClass { @@ -920,6 +916,11 @@ vips_foreign_load_heif_get_position( void *userdata ) return( vips_source_seek( heif->source, 0L, SEEK_CUR ) ); } +/* libheif read() does not work like unix read(). + * + * This method is cannot return EOF. Instead, the separate wait_for_file_size() + * is called beforehand to make sure that there's enough data there. + */ static int vips_foreign_load_heif_read( void *data, size_t size, void *userdata ) { @@ -928,16 +929,6 @@ vips_foreign_load_heif_read( void *data, size_t size, void *userdata ) gint64 result; result = vips_source_read( heif->source, data, size ); - /* On EOF, make a note of the file length. - * - * libheif can sometimes ask for zero bytes, be careful not to - * interpret that as EOF. - */ - if( size > 0 && - result == 0 && - heif->length == -1 ) - result = heif->length = - vips_source_seek( heif->source, 0L, SEEK_CUR ); if( result < 0 ) return( -1 ); @@ -954,30 +945,33 @@ vips_foreign_load_heif_seek( gint64 position, void *userdata ) return( 0 ); } +/* libheif calls this to mean "I intend to read() to this position, please + * check it is OK". + */ static enum heif_reader_grow_status vips_foreign_load_heif_wait_for_file_size( gint64 target_size, void *userdata ) { VipsForeignLoadHeif *heif = (VipsForeignLoadHeif *) userdata; + gint64 old_position; + gint64 result; enum heif_reader_grow_status status; - if( heif->source->data != NULL && target_size > heif->source->length ) - /* Target size is beyond known buffer length + /* We seek the VipsSource to the position and check for errors. + */ + old_position = vips_source_seek( heif->source, 0L, SEEK_CUR ); + result = vips_source_seek( heif->source, target_size, SEEK_SET ); + vips_source_seek( heif->source, old_position, SEEK_SET ); + + if( result < 0 ) + /* Unable to seek to this point, so it's beyond EOF. */ status = heif_reader_grow_status_size_beyond_eof; - else if( heif->length == -1 ) - /* We've not seen EOF yet, so seeking to any point is fine (as - * far as we know). - */ - status = heif_reader_grow_status_size_reached; - else if( target_size <= heif->length ) - /* We've seen EOF, and this target is less than that. - */ - status = heif_reader_grow_status_size_reached; else - /* We've seen EOF, we know the length, and this is too far. + /* Successfully read to the requested point, but the requested + * point is not necessarily EOF. */ - status = heif_reader_grow_status_size_beyond_eof; + status = heif_reader_grow_status_size_reached; return( status ); } @@ -986,7 +980,6 @@ static void vips_foreign_load_heif_init( VipsForeignLoadHeif *heif ) { heif->n = 1; - heif->length = -1; heif->reader = VIPS_ARRAY( NULL, 1, struct heif_reader );