From a46c111d9f3642f0ef3819e7298846ccc61869e0 Mon Sep 17 00:00:00 2001 From: DRC Date: Mon, 27 Jul 2020 14:21:23 -0500 Subject: [PATCH] Further jpeg_skip_scanlines() fixes - Introduce a partial image decompression regression test script that validates the correctness of jpeg_skip_scanlines() and jpeg_crop_scanlines() for a variety of cropping regions and libjpeg settings. This regression test catches the following issues: #182, fixed in 5bc43c7 #237, fixed in 6e95c08 #244, fixed in 398c1e9 #441, fully fixed in this commit It does not catch the following issues: #194, fixed in 773040f #244 (additional segfault), fixed in 9120a24 - Modify the libjpeg-turbo regression test suite (make test) so that it checks for the issue reported in #441 (segfault in jpeg_skip_scanlines() when used with 4:2:0 merged upsampling/color conversion.) - Fix issues in jpeg_skip_scanlines() that caused incorrect output with h2v2 (4:2:0) merged upsampling/color conversion. The previous commit fixed the segfault reported in #441, but that was a symptom of a larger problem. Because merged 4:2:0 upsampling uses a "spare row" buffer, it is necessary to allow the upsampler to run when skipping rows (fancy 4:2:0 upsampling, which uses context rows, also requires this.) Otherwise, if skipping starts at an odd-numbered row, the output image will be incorrect. - Throw an error if jpeg_skip_scanlines() is called with two-pass color quantization enabled. With two-pass color quantization, the first pass occurs within jpeg_start_decompress(), so subsequent calls to jpeg_skip_scanlines() interfere with the multipass state and prevent the second pass from occurring during subsequent calls to jpeg_read_scanlines(). Upstream-Status: Backport [https://github.com/libjpeg-turbo/libjpeg-turbo/commit/a46c111d9f3642f0ef3819e7298846ccc61869e0] CVE: CVE-2020-35538 Signed-off-by: Vijay Anusuri --- CMakeLists.txt | 9 +++-- ChangeLog.md | 15 +++++--- croptest.in | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++ jdapistd.c | 70 +++++++++++-------------------------- libjpeg.txt | 6 ++-- 5 files changed, 136 insertions(+), 59 deletions(-) create mode 100755 croptest.in diff --git a/CMakeLists.txt b/CMakeLists.txt index aee74c9..de451f4 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -753,7 +753,7 @@ else() set(MD5_PPM_3x2_IFAST fd283664b3b49127984af0a7f118fccd) set(MD5_JPEG_420_ISLOW_ARI e986fb0a637a8d833d96e8a6d6d84ea1) set(MD5_JPEG_444_ISLOW_PROGARI 0a8f1c8f66e113c3cf635df0a475a617) - set(MD5_PPM_420M_IFAST_ARI 72b59a99bcf1de24c5b27d151bde2437) + set(MD5_PPM_420M_IFAST_ARI 57251da28a35b46eecb7177d82d10e0e) set(MD5_JPEG_420_ISLOW 9a68f56bc76e466aa7e52f415d0f4a5f) set(MD5_PPM_420M_ISLOW_2_1 9f9de8c0612f8d06869b960b05abf9c9) set(MD5_PPM_420M_ISLOW_15_8 b6875bc070720b899566cc06459b63b7) @@ -1131,7 +1131,7 @@ foreach(libtype ${TEST_LIBTYPES}) if(WITH_ARITH_DEC) # CC: RGB->YCC SAMP: h2v2 merged IDCT: ifast ENT: arith - add_bittest(djpeg 420m-ifast-ari "-fast;-ppm" + add_bittest(djpeg 420m-ifast-ari "-fast;-skip;1,20;-ppm" testout_420m_ifast_ari.ppm ${TESTIMAGES}/testimgari.jpg ${MD5_PPM_420M_IFAST_ARI}) @@ -1266,6 +1266,11 @@ endforeach() add_custom_target(testclean COMMAND ${CMAKE_COMMAND} -P ${CMAKE_CURRENT_SOURCE_DIR}/cmakescripts/testclean.cmake) +configure_file(croptest.in croptest @ONLY) +add_custom_target(croptest + COMMAND echo croptest + COMMAND ${BASH} ${CMAKE_CURRENT_BINARY_DIR}/croptest) + if(WITH_TURBOJPEG) configure_file(tjbenchtest.in tjbenchtest @ONLY) configure_file(tjexampletest.in tjexampletest @ONLY) diff --git a/ChangeLog.md b/ChangeLog.md index 19d18fa..4562eff 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -54,11 +54,16 @@ a 16-bit binary PGM file into an RGB image buffer. generated when using the `tjLoadImage()` function to load a 16-bit binary PPM file into an extended RGB image buffer. -2. Fixed segfaults or "Corrupt JPEG data: premature end of data segment" errors -in `jpeg_skip_scanlines()` that occurred when decompressing 4:2:2 or 4:2:0 JPEG -images using the merged (non-fancy) upsampling algorithms (that is, when -setting `cinfo.do_fancy_upsampling` to `FALSE`.) 2.0.0[6] was a similar fix, -but it did not cover all cases. +2. Fixed or worked around multiple issues with `jpeg_skip_scanlines()`: + + - Fixed segfaults or "Corrupt JPEG data: premature end of data segment" +errors in `jpeg_skip_scanlines()` that occurred when decompressing 4:2:2 or +4:2:0 JPEG images using merged (non-fancy) upsampling/color conversion (that +is, when setting `cinfo.do_fancy_upsampling` to `FALSE`.) 2.0.0[6] was a +similar fix, but it did not cover all cases. + - `jpeg_skip_scanlines()` now throws an error if two-pass color +quantization is enabled. Two-pass color quantization never worked properly +with `jpeg_skip_scanlines()`, and the issues could not readily be fixed. 2.0.3 diff --git a/croptest.in b/croptest.in new file mode 100755 index 0000000..7e3c293 --- /dev/null +++ b/croptest.in @@ -0,0 +1,95 @@ +#!/bin/bash + +set -u +set -e +trap onexit INT +trap onexit TERM +trap onexit EXIT + +onexit() +{ + if [ -d $OUTDIR ]; then + rm -rf $OUTDIR + fi +} + +runme() +{ + echo \*\*\* $* + $* +} + +IMAGE=vgl_6548_0026a.bmp +WIDTH=128 +HEIGHT=95 +IMGDIR=@CMAKE_CURRENT_SOURCE_DIR@/testimages +OUTDIR=`mktemp -d /tmp/__croptest_output.XXXXXX` +EXEDIR=@CMAKE_CURRENT_BINARY_DIR@ + +if [ -d $OUTDIR ]; then + rm -rf $OUTDIR +fi +mkdir -p $OUTDIR + +exec >$EXEDIR/croptest.log + +echo "============================================================" +echo "$IMAGE ($WIDTH x $HEIGHT)" +echo "============================================================" +echo + +for PROGARG in "" -progressive; do + + cp $IMGDIR/$IMAGE $OUTDIR + basename=`basename $IMAGE .bmp` + echo "------------------------------------------------------------" + echo "Generating test images" + echo "------------------------------------------------------------" + echo + runme $EXEDIR/cjpeg $PROGARG -grayscale -outfile $OUTDIR/${basename}_GRAY.jpg $IMGDIR/${basename}.bmp + runme $EXEDIR/cjpeg $PROGARG -sample 2x2 -outfile $OUTDIR/${basename}_420.jpg $IMGDIR/${basename}.bmp + runme $EXEDIR/cjpeg $PROGARG -sample 2x1 -outfile $OUTDIR/${basename}_422.jpg $IMGDIR/${basename}.bmp + runme $EXEDIR/cjpeg $PROGARG -sample 1x2 -outfile $OUTDIR/${basename}_440.jpg $IMGDIR/${basename}.bmp + runme $EXEDIR/cjpeg $PROGARG -sample 1x1 -outfile $OUTDIR/${basename}_444.jpg $IMGDIR/${basename}.bmp + echo + + for NSARG in "" -nosmooth; do + + for COLORSARG in "" "-colors 256 -dither none -onepass"; do + + for Y in {0..16}; do + + for H in {1..16}; do + + X=$(( (Y*16)%128 )) + W=$(( WIDTH-X-7 )) + if [ $Y -le 15 ]; then + CROPSPEC="${W}x${H}+${X}+${Y}" + else + Y2=$(( HEIGHT-H )); + CROPSPEC="${W}x${H}+${X}+${Y2}" + fi + + echo "------------------------------------------------------------" + echo $PROGARG $NSARG $COLORSARG -crop $CROPSPEC + echo "------------------------------------------------------------" + echo + for samp in GRAY 420 422 440 444; do + $EXEDIR/djpeg $NSARG $COLORSARG -rgb -outfile $OUTDIR/${basename}_${samp}_full.ppm $OUTDIR/${basename}_${samp}.jpg + convert -crop $CROPSPEC $OUTDIR/${basename}_${samp}_full.ppm $OUTDIR/${basename}_${samp}_ref.ppm + runme $EXEDIR/djpeg $NSARG $COLORSARG -crop $CROPSPEC -rgb -outfile $OUTDIR/${basename}_${samp}.ppm $OUTDIR/${basename}_${samp}.jpg + runme cmp $OUTDIR/${basename}_${samp}.ppm $OUTDIR/${basename}_${samp}_ref.ppm + done + echo + + done + + done + + done + + done + +done + +echo SUCCESS! diff --git a/jdapistd.c b/jdapistd.c index 91da642..c502909 100644 --- a/jdapistd.c +++ b/jdapistd.c @@ -306,16 +306,6 @@ noop_quantize(j_decompress_ptr cinfo, JSAMPARRAY input_buf, } -/* Dummy postprocessing function used by jpeg_skip_scanlines() */ -LOCAL(void) -noop_post_process (j_decompress_ptr cinfo, JSAMPIMAGE input_buf, - JDIMENSION *in_row_group_ctr, - JDIMENSION in_row_groups_avail, JSAMPARRAY output_buf, - JDIMENSION *out_row_ctr, JDIMENSION out_rows_avail) -{ -} - - /* * In some cases, it is best to call jpeg_read_scanlines() and discard the * output, rather than skipping the scanlines, because this allows us to @@ -329,16 +319,12 @@ read_and_discard_scanlines(j_decompress_ptr cinfo, JDIMENSION num_lines) { JDIMENSION n; my_master_ptr master = (my_master_ptr)cinfo->master; + JSAMPARRAY scanlines = NULL; void (*color_convert) (j_decompress_ptr cinfo, JSAMPIMAGE input_buf, JDIMENSION input_row, JSAMPARRAY output_buf, int num_rows) = NULL; void (*color_quantize) (j_decompress_ptr cinfo, JSAMPARRAY input_buf, JSAMPARRAY output_buf, int num_rows) = NULL; - void (*post_process_data) (j_decompress_ptr cinfo, JSAMPIMAGE input_buf, - JDIMENSION *in_row_group_ctr, - JDIMENSION in_row_groups_avail, - JSAMPARRAY output_buf, JDIMENSION *out_row_ctr, - JDIMENSION out_rows_avail) = NULL; if (cinfo->cconvert && cinfo->cconvert->color_convert) { color_convert = cinfo->cconvert->color_convert; @@ -350,23 +336,19 @@ read_and_discard_scanlines(j_decompress_ptr cinfo, JDIMENSION num_lines) cinfo->cquantize->color_quantize = noop_quantize; } - if (master->using_merged_upsample && cinfo->post && - cinfo->post->post_process_data) { - post_process_data = cinfo->post->post_process_data; - cinfo->post->post_process_data = noop_post_process; + if (master->using_merged_upsample && cinfo->max_v_samp_factor == 2) { + my_merged_upsample_ptr upsample = (my_merged_upsample_ptr)cinfo->upsample; + scanlines = &upsample->spare_row; } for (n = 0; n < num_lines; n++) - jpeg_read_scanlines(cinfo, NULL, 1); + jpeg_read_scanlines(cinfo, scanlines, 1); if (color_convert) cinfo->cconvert->color_convert = color_convert; if (color_quantize) cinfo->cquantize->color_quantize = color_quantize; - - if (post_process_data) - cinfo->post->post_process_data = post_process_data; } @@ -380,6 +362,12 @@ increment_simple_rowgroup_ctr(j_decompress_ptr cinfo, JDIMENSION rows) { JDIMENSION rows_left; my_main_ptr main_ptr = (my_main_ptr)cinfo->main; + my_master_ptr master = (my_master_ptr)cinfo->master; + + if (master->using_merged_upsample && cinfo->max_v_samp_factor == 2) { + read_and_discard_scanlines(cinfo, rows); + return; + } /* Increment the counter to the next row group after the skipped rows. */ main_ptr->rowgroup_ctr += rows / cinfo->max_v_samp_factor; @@ -410,11 +398,16 @@ jpeg_skip_scanlines(j_decompress_ptr cinfo, JDIMENSION num_lines) my_main_ptr main_ptr = (my_main_ptr)cinfo->main; my_coef_ptr coef = (my_coef_ptr)cinfo->coef; my_master_ptr master = (my_master_ptr)cinfo->master; + my_upsample_ptr upsample = (my_upsample_ptr)cinfo->upsample; JDIMENSION i, x; int y; JDIMENSION lines_per_iMCU_row, lines_left_in_iMCU_row, lines_after_iMCU_row; JDIMENSION lines_to_skip, lines_to_read; + /* Two-pass color quantization is not supported. */ + if (cinfo->quantize_colors && cinfo->two_pass_quantize) + ERREXIT(cinfo, JERR_NOTIMPL); + if (cinfo->global_state != DSTATE_SCANNING) ERREXIT1(cinfo, JERR_BAD_STATE, cinfo->global_state); @@ -472,13 +465,7 @@ jpeg_skip_scanlines(j_decompress_ptr cinfo, JDIMENSION num_lines) main_ptr->buffer_full = FALSE; main_ptr->rowgroup_ctr = 0; main_ptr->context_state = CTX_PREPARE_FOR_IMCU; - if (master->using_merged_upsample) { - my_merged_upsample_ptr upsample = - (my_merged_upsample_ptr)cinfo->upsample; - upsample->spare_full = FALSE; - upsample->rows_to_go = cinfo->output_height - cinfo->output_scanline; - } else { - my_upsample_ptr upsample = (my_upsample_ptr)cinfo->upsample; + if (!master->using_merged_upsample) { upsample->next_row_out = cinfo->max_v_samp_factor; upsample->rows_to_go = cinfo->output_height - cinfo->output_scanline; } @@ -493,13 +480,7 @@ jpeg_skip_scanlines(j_decompress_ptr cinfo, JDIMENSION num_lines) cinfo->output_scanline += lines_left_in_iMCU_row; main_ptr->buffer_full = FALSE; main_ptr->rowgroup_ctr = 0; - if (master->using_merged_upsample) { - my_merged_upsample_ptr upsample = - (my_merged_upsample_ptr)cinfo->upsample; - upsample->spare_full = FALSE; - upsample->rows_to_go = cinfo->output_height - cinfo->output_scanline; - } else { - my_upsample_ptr upsample = (my_upsample_ptr)cinfo->upsample; + if (!master->using_merged_upsample) { upsample->next_row_out = cinfo->max_v_samp_factor; upsample->rows_to_go = cinfo->output_height - cinfo->output_scanline; } @@ -537,14 +518,8 @@ jpeg_skip_scanlines(j_decompress_ptr cinfo, JDIMENSION num_lines) cinfo->output_iMCU_row += lines_to_skip / lines_per_iMCU_row; increment_simple_rowgroup_ctr(cinfo, lines_to_read); } - if (master->using_merged_upsample) { - my_merged_upsample_ptr upsample = - (my_merged_upsample_ptr)cinfo->upsample; - upsample->rows_to_go = cinfo->output_height - cinfo->output_scanline; - } else { - my_upsample_ptr upsample = (my_upsample_ptr)cinfo->upsample; + if (!master->using_merged_upsample) upsample->rows_to_go = cinfo->output_height - cinfo->output_scanline; - } return num_lines; } @@ -585,13 +560,8 @@ jpeg_skip_scanlines(j_decompress_ptr cinfo, JDIMENSION num_lines) * bit odd, since "rows_to_go" seems to be redundantly keeping track of * output_scanline. */ - if (master->using_merged_upsample) { - my_merged_upsample_ptr upsample = (my_merged_upsample_ptr)cinfo->upsample; + if (!master->using_merged_upsample) upsample->rows_to_go = cinfo->output_height - cinfo->output_scanline; - } else { - my_upsample_ptr upsample = (my_upsample_ptr)cinfo->upsample; - upsample->rows_to_go = cinfo->output_height - cinfo->output_scanline; - } /* Always skip the requested number of lines. */ return num_lines; diff --git a/libjpeg.txt b/libjpeg.txt index c50cf90..c233ecb 100644 --- a/libjpeg.txt +++ b/libjpeg.txt @@ -3,7 +3,7 @@ USING THE IJG JPEG LIBRARY This file was part of the Independent JPEG Group's software: Copyright (C) 1994-2013, Thomas G. Lane, Guido Vollbeding. libjpeg-turbo Modifications: -Copyright (C) 2010, 2014-2018, D. R. Commander. +Copyright (C) 2010, 2014-2018, 2020, D. R. Commander. Copyright (C) 2015, Google, Inc. For conditions of distribution and use, see the accompanying README.ijg file. @@ -750,7 +750,9 @@ multiple rows in the JPEG image. Suspending data sources are not supported by this function. Calling jpeg_skip_scanlines() with a suspending data source will result in undefined -behavior. +behavior. Two-pass color quantization is also not supported by this function. +Calling jpeg_skip_scanlines() with two-pass color quantization enabled will +result in an error. jpeg_skip_scanlines() will not allow skipping past the bottom of the image. If the value of num_lines is large enough to skip past the bottom of the image, -- 2.25.1