djpeg/jpeg_crop_scanline(): Disallow crop vals < 0

Because the crop spec was parsed using unsigned 32-bit integers,
negative numbers were interpreted as values ~= UINT_MAX (4,294,967,295).
This had the following ramifications:

- If the cropping region width was negative and the adjusted width + the
  adjusted left boundary was greater than 0, then the 32-bit unsigned
  integer bounds checks in djpeg and jpeg_crop_scanline() overflowed and
  failed to detect the out-of-bounds width, jpeg_crop_scanline() set
  cinfo->output_width to a value ~= UINT_MAX, and a buffer overrun and
  subsequent segfault occurred in the upsampling or color conversion
  routine.  The segfault occurred in the body of
  jpeg_skip_scanlines() --> read_and_discard_scanlines() if the cropping
  region upper boundary was greater than 0 and the JPEG image used
  chrominance subsampling and in the body of jpeg_read_scanlines()
  otherwise.

- If the cropping region width was negative and the adjusted width + the
  adjusted left boundary was 0, then a zero-width output image was
  generated.

- If the cropping region left boundary was negative, then an output
  image with bogus data was generated.

This commit modifies djpeg and jpeg_crop_scanline() so that the
aforementioned bounds checks use 64-bit unsigned integers, thus guarding
against overflow.  It similarly modifies jpeg_skip_scanlines().  In the
case of jpeg_skip_scanlines(), the issue was not reproducible with
djpeg, but passing a negative number of lines to jpeg_skip_scanlines()
caused a similar overflow if the number of lines +
cinfo->output_scanline was greater than 0.  That caused
jpeg_skip_scanlines() to read past the end of the JPEG image, throw a
warning ("Corrupt JPEG data: premature end of data segment"), and fail
to return unless warnings were treated as fatal.  Also, djpeg now parses
the crop spec using signed integers and checks for negative values.
This commit is contained in:
DRC
2024-08-26 12:14:20 -04:00
parent 548f732432
commit 4851cbe406
3 changed files with 31 additions and 10 deletions

View File

@@ -33,6 +33,16 @@ attempting to generate a full-color lossless JPEG image using the TurboJPEG
Java API's `byte[] TJCompressor.compress()` method if the value of Java API's `byte[] TJCompressor.compress()` method if the value of
`TJ.PARAM_SUBSAMP` was not `TJ.SAMP_444`. `TJ.PARAM_SUBSAMP` was not `TJ.SAMP_444`.
6. Fixed a segfault in djpeg that occurred if a negative width was specified
with the `-crop` option. Since the cropping region width was read into an
unsigned 32-bit integer, a negative width was interpreted as a very large
value. With certain negative width and positive left boundary values, the
bounds checks in djpeg and `jpeg_crop_scanline()` overflowed and did not detect
the out-of-bounds width, which caused a buffer overrun in the upsampling or
color conversion routine. Both bounds checks now use 64-bit integers to guard
against overflow, and djpeg now checks for negative numbers when it parses the
crop specification from the command line.
3.0.3 3.0.3
===== =====

23
djpeg.c
View File

@@ -401,22 +401,31 @@ parse_switches(j_decompress_ptr cinfo, int argc, char **argv,
usage(); usage();
} else if (keymatch(arg, "skip", 2)) { } else if (keymatch(arg, "skip", 2)) {
int temp_start = -1, temp_end = -1;
if (++argn >= argc) if (++argn >= argc)
usage(); usage();
if (sscanf(argv[argn], "%u,%u", &skip_start, &skip_end) != 2 || if (sscanf(argv[argn], "%d,%d", &temp_start, &temp_end) != 2 ||
skip_start > skip_end) temp_start < 0 || temp_end < 0 || temp_start > temp_end)
usage(); usage();
skip = TRUE; skip = TRUE;
skip_start = temp_start;
skip_end = temp_end;
} else if (keymatch(arg, "crop", 2)) { } else if (keymatch(arg, "crop", 2)) {
int temp_width = -1, temp_height = -1, temp_x = -1, temp_y = -1;
char c; char c;
if (++argn >= argc) if (++argn >= argc)
usage(); usage();
if (sscanf(argv[argn], "%u%c%u+%u+%u", &crop_width, &c, &crop_height, if (sscanf(argv[argn], "%d%c%d+%d+%d", &temp_width, &c, &temp_height,
&crop_x, &crop_y) != 5 || &temp_x, &temp_y) != 5 ||
(c != 'X' && c != 'x') || crop_width < 1 || crop_height < 1) (c != 'X' && c != 'x') || temp_width < 1 || temp_height < 1 ||
temp_x < 0 || temp_y < 0)
usage(); usage();
crop = TRUE; crop = TRUE;
crop_width = temp_width;
crop_height = temp_height;
crop_x = temp_x;
crop_y = temp_y;
} else if (keymatch(arg, "strict", 2)) { } else if (keymatch(arg, "strict", 2)) {
strict = TRUE; strict = TRUE;
@@ -776,8 +785,8 @@ main(int argc, char **argv)
/* Check for valid crop dimensions. We cannot check these values until /* Check for valid crop dimensions. We cannot check these values until
* after jpeg_start_decompress() is called. * after jpeg_start_decompress() is called.
*/ */
if (crop_x + crop_width > cinfo.output_width || if ((unsigned long long)crop_x + crop_width > cinfo.output_width ||
crop_y + crop_height > cinfo.output_height) { (unsigned long long)crop_y + crop_height > cinfo.output_height) {
fprintf(stderr, "%s: crop dimensions exceed image dimensions %u x %u\n", fprintf(stderr, "%s: crop dimensions exceed image dimensions %u x %u\n",
progname, cinfo.output_width, cinfo.output_height); progname, cinfo.output_width, cinfo.output_height);
exit(EXIT_FAILURE); exit(EXIT_FAILURE);

View File

@@ -4,7 +4,7 @@
* This file was part of the Independent JPEG Group's software: * This file was part of the Independent JPEG Group's software:
* Copyright (C) 1994-1996, Thomas G. Lane. * Copyright (C) 1994-1996, Thomas G. Lane.
* libjpeg-turbo Modifications: * libjpeg-turbo Modifications:
* Copyright (C) 2010, 2015-2020, 2022-2023, D. R. Commander. * Copyright (C) 2010, 2015-2020, 2022-2024, D. R. Commander.
* Copyright (C) 2015, Google, Inc. * Copyright (C) 2015, Google, Inc.
* For conditions of distribution and use, see the accompanying README.ijg * For conditions of distribution and use, see the accompanying README.ijg
* file. * file.
@@ -200,7 +200,8 @@ _jpeg_crop_scanline(j_decompress_ptr cinfo, JDIMENSION *xoffset,
ERREXIT(cinfo, JERR_BAD_CROP_SPEC); ERREXIT(cinfo, JERR_BAD_CROP_SPEC);
/* xoffset and width must fall within the output image dimensions. */ /* xoffset and width must fall within the output image dimensions. */
if (*width == 0 || *xoffset + *width > cinfo->output_width) if (*width == 0 ||
(unsigned long long)(*xoffset) + *width > cinfo->output_width)
ERREXIT(cinfo, JERR_WIDTH_OVERFLOW); ERREXIT(cinfo, JERR_WIDTH_OVERFLOW);
/* No need to do anything if the caller wants the entire width. */ /* No need to do anything if the caller wants the entire width. */
@@ -482,7 +483,8 @@ _jpeg_skip_scanlines(j_decompress_ptr cinfo, JDIMENSION num_lines)
ERREXIT1(cinfo, JERR_BAD_STATE, cinfo->global_state); ERREXIT1(cinfo, JERR_BAD_STATE, cinfo->global_state);
/* Do not skip past the bottom of the image. */ /* Do not skip past the bottom of the image. */
if (cinfo->output_scanline + num_lines >= cinfo->output_height) { if ((unsigned long long)cinfo->output_scanline + num_lines >=
cinfo->output_height) {
num_lines = cinfo->output_height - cinfo->output_scanline; num_lines = cinfo->output_height - cinfo->output_scanline;
cinfo->output_scanline = cinfo->output_height; cinfo->output_scanline = cinfo->output_height;
(*cinfo->inputctl->finish_input_pass) (cinfo); (*cinfo->inputctl->finish_input_pass) (cinfo);