From 10003f3f3cbc0fd98d705fbd9ffbea59c820b299 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Fri, 25 Aug 2017 12:51:06 +0100 Subject: [PATCH] revise libjpeg new from buffer in line with latest libjpeg recommendations this fixes a segv with corrupt input found by libFuzzer and asan --- libvips/foreign/jpeg2vips.c | 80 ++++++++++++------------------------- 1 file changed, 25 insertions(+), 55 deletions(-) diff --git a/libvips/foreign/jpeg2vips.c b/libvips/foreign/jpeg2vips.c index 58fed70b..1d11a7d2 100644 --- a/libvips/foreign/jpeg2vips.c +++ b/libvips/foreign/jpeg2vips.c @@ -87,6 +87,9 @@ * - invalidate operation on read error * 12/5/17 * - fail aborts on error, not warning + * 25/8/17 + * - revise read from buffer functions in line with latest libjpeg + * recommendations -- fixes a segv with crafted jpeg images */ /* @@ -776,59 +779,21 @@ typedef struct { /* Private stuff during read. */ - gboolean start_of_file; /* have we gotten any data yet? */ const JOCTET *buf; size_t len; } InputBuffer; -/* - * Initialize source --- called by jpeg_read_header - * before any data is actually read. - */ - static void init_source (j_decompress_ptr cinfo) { - InputBuffer *src = (InputBuffer *) cinfo->src; - - /* We reset the empty-input-file flag for each image, - * but we don't clear the input buffer. - * This is correct behavior for reading a series of images from one source. - */ - src->start_of_file = TRUE; + /* no work necessary here */ } /* * Fill the input buffer --- called whenever buffer is emptied. * - * In typical applications, this should read fresh data into the buffer - * (ignoring the current state of next_input_byte & bytes_in_buffer), - * reset the pointer & count to the start of the buffer, and return TRUE - * indicating that the buffer has been reloaded. It is not necessary to - * fill the buffer entirely, only to obtain at least one more byte. - * - * There is no such thing as an EOF return. If the end of the file has been - * reached, the routine has a choice of ERREXIT() or inserting fake data into - * the buffer. In most cases, generating a warning message and inserting a - * fake EOI marker is the best course of action --- this will allow the - * decompressor to output however much of the image is there. However, - * the resulting error message is misleading if the real problem is an empty - * input file, so we handle that case specially. - * - * In applications that need to be able to suspend compression due to input - * not being available yet, a FALSE return indicates that no more data can be - * obtained right now, but more may be forthcoming later. In this situation, - * the decompressor will return to its caller (with an indication of the - * number of scanlines it has read, if any). The application should resume - * decompression after it has loaded more data into the input buffer. Note - * that there are substantial restrictions on the use of suspension --- see - * the documentation. - * - * When suspending, the decompressor will back up to a convenient restart point - * (typically the start of the current MCU). next_input_byte & bytes_in_buffer - * indicate where the restart point will be if the current call returns FALSE. - * Data beyond this point must be rescanned after resumption, so move it to - * the front of the buffer rather than discarding it. + * We fill the buffer on readjpeg_buffer(), so this will only be called if + * libjpeg tries to read beyond the buffer. */ static boolean @@ -840,16 +805,9 @@ fill_input_buffer (j_decompress_ptr cinfo) InputBuffer *src = (InputBuffer *) cinfo->src; - if (src->start_of_file && src->len > 0) { - src->pub.next_input_byte = src->buf; - src->pub.bytes_in_buffer = src->len; - src->start_of_file = FALSE; - } - else { - WARNMS(cinfo, JWRN_JPEG_EOF); - src->pub.next_input_byte = eoi_buffer; - src->pub.bytes_in_buffer = 2; - } + WARNMS(cinfo, JWRN_JPEG_EOF); + src->pub.next_input_byte = eoi_buffer; + src->pub.bytes_in_buffer = 2; return TRUE; } @@ -871,9 +829,16 @@ skip_input_data (j_decompress_ptr cinfo, long num_bytes) { InputBuffer *src = (InputBuffer *) cinfo->src; - /* Just skip fwd. - */ if (num_bytes > 0) { + while (num_bytes > (long) src->pub.bytes_in_buffer) { + num_bytes -= (long) src->pub.bytes_in_buffer; + (void) (*src->pub.fill_input_buffer) (cinfo); + + /* note we assume that fill_input_buffer will never return FALSE, + * so suspension need not be handled. + */ + } + src->pub.next_input_byte += (size_t) num_bytes; src->pub.bytes_in_buffer -= (size_t) num_bytes; } @@ -913,6 +878,11 @@ readjpeg_buffer (ReadJpeg *jpeg, const void *buf, size_t len) j_decompress_ptr cinfo = &jpeg->cinfo; InputBuffer *src; + /* Empty buffer is a fatal error. + */ + if (len == 0 || buf == NULL) + ERREXIT(cinfo, JERR_INPUT_EMPTY); + /* The source object and input buffer are made permanent so that a series * of JPEG images can be read from the same file by calling jpeg_stdio_src * only before the first one. (If we discarded the buffer at the end of @@ -935,8 +905,8 @@ readjpeg_buffer (ReadJpeg *jpeg, const void *buf, size_t len) src->pub.skip_input_data = skip_input_data; src->pub.resync_to_restart = jpeg_resync_to_restart; /* use default method */ src->pub.term_source = term_source; - src->pub.bytes_in_buffer = 0; /* forces fill_input_buffer on first read */ - src->pub.next_input_byte = NULL; /* until buffer loaded */ + src->pub.bytes_in_buffer = len; + src->pub.next_input_byte = buf; } int